Skip to content

Conversation

@maggch97
Copy link
Contributor

@maggch97 maggch97 commented Aug 4, 2022

No description provided.

@maggch97
Copy link
Contributor Author

maggch97 commented Aug 4, 2022

these api are available in tampermonkey and used by my scripts,so add these to userscripts also.

@quoid
Copy link
Owner

quoid commented Aug 4, 2022

Thanks for the PR @maggch97 !

I've not used this API in the past. What's the main use case for it?

It looks like it's so that usescripts can share data with one another, this correct?

@maggch97
Copy link
Contributor Author

maggch97 commented Aug 5, 2022

Thanks for the PR @maggch97 !

I've not used this API in the past. What's the main use case for it?

It looks like it's so that usescripts can share data with one another, this correct?

for my usage,i use this api to persist the state for tab. if user refresh the page or jump to new page with different host,script can recover from the previous tan state.

@quoid
Copy link
Owner

quoid commented Aug 5, 2022

Thanks for the clarification. I just want to make sure on one more thing. In the readme you say:

on success returns a promise resolved with a object that is persistent as long as this tab is open

On tab close this object should be cleared?

@maggch97
Copy link
Contributor Author

maggch97 commented Aug 5, 2022

according to the implementation in tampermonkey, we don't need to clear the object.

https://github.com/Tampermonkey/tampermonkey/blob/07f668cd1cabb2939220045839dec4d95d2db0c8/src/background.js#L65

@quoid
Copy link
Owner

quoid commented Aug 5, 2022

@maggch97

right, but we can clear the object when tab closes?

Tampermonkey has a persistent background page, Userscripts has a non-persistent background page (this is an iOS requirement). So we should not store US_tabs in a global var since it can be cleared at any moment when the background page is unloaded.

I think sessionStorage is a good place, but sessionStorage can get cleared when tab closes. I want to make sure that clearing the object on tab close won't limit functionality.

@maggch97
Copy link
Contributor Author

maggch97 commented Aug 5, 2022

@maggch97

right, but we can clear the object when tab closes?

Tampermonkey has a persistent background page, Userscripts has a non-persistent background page (this is an iOS requirement). So we should not store US_tabs in a global var since it can be cleared at any moment when the background page is unloaded.

I think sessionStorage is a good place, but sessionStorage can get cleared when tab closes. I want to make sure that clearing the object on tab close won't limit functionality.

Thanks for your information because i am not familiar with Safari and iOS,please let me do some investigation

@maggch97
Copy link
Contributor Author

maggch97 commented Aug 5, 2022

@maggch97

right, but we can clear the object when tab closes?

Tampermonkey has a persistent background page, Userscripts has a non-persistent background page (this is an iOS requirement). So we should not store US_tabs in a global var since it can be cleared at any moment when the background page is unloaded.

I think sessionStorage is a good place, but sessionStorage can get cleared when tab closes. I want to make sure that clearing the object on tab close won't limit functionality.

but I don't think sessionStorage can be used here,because pages from different hosts like google and github can't share their sessionStorage. Howerver,We can open different host pages in a same tab one by one

@quoid
Copy link
Owner

quoid commented Aug 5, 2022

but I don't think sessionStorage can be used here,because pages from different hosts like google and github can't share their sessionStorage. Howerver,We can open different host pages in a same tab one by one

We could use the sessionStorage from the background page. It will function like a global var, but persist when the background page is unloaded This should not have the same restrictions as page scoped sessionStorage.

The background sessionStorage is also cleared when the browser closes (not window, not tab), so this would be better than having data persist indefinitely in localStorage - also, localStorage could introduce problems since tab ids are only unique throughout sessions.

It would be ideal to clear the tab data when the tab is closed, but I am not sure it's worth have browser.tabs.onRemoved listener constantly running for this api. I think using sessionStorage would be good enough.

@quoid
Copy link
Owner

quoid commented Aug 6, 2022

@maggch97 I made the changes to your pull request, below are noted on what I did, please let me know if any of the changes conflict with the API, thanks again for the contribution:

  • save tab data to sessionStorage of the background page
    • initial testing seems to indicate this is a valid approach
    • gets cleared on session end
  • saveTab can now save Any as indicated in the readme change, originally only objects {} would be saved successfully
    • the user can now save, string, objects, arrays, etc...
    • please let me know if saveTab should only accept objects {key: val}
  • added the new method names to the utils.js file, originally you added it to page.js which is where utlis.js gets compiled to
    • when compiling a new version you can use npm run build:page or npm run build:popup and generally shouldn't ever directly edit page.js, popup.js, popup.html or page.html
  • some formatting cleanup

Copy link
Owner

@quoid quoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor changes needed but I added additional commits

@maggch97
Copy link
Contributor Author

maggch97 commented Aug 6, 2022

@quoid sorry, I didn't see your comment before and force push now. I think you changes won't conflict with the API

@quoid
Copy link
Owner

quoid commented Aug 6, 2022

@quoid sorry, I didn't see your comment before and force push now. I think you changes won't conflict with the API

@maggch97 no problem, i'll fix it

@quoid quoid changed the base branch from master to v4.2.3 August 6, 2022 17:12
@quoid
Copy link
Owner

quoid commented Aug 6, 2022

merging this into the 4.2.3 branch

Repository owner deleted a comment Aug 6, 2022
@quoid quoid merged commit 0b33242 into quoid:v4.2.3 Aug 6, 2022
@ACTCD
Copy link
Collaborator

ACTCD commented Jul 8, 2024

I'm sorry, but I have to revisit the existence of these APIs today.

First of all, I must admit that I don't like this set of APIs at all.

It's very unintuitive for both developers and users, and it takes me some time to re-understand it every time I see it.

First is the name, which is actually more like getTabBindData and saveTabBindData than what it appears now.

Secondly, I understand that its function is actually to track tabs across origins, which means that it actually only needs an identifier like tabID, and any logic can be completed using the existing GM.*Value APIs.

And GM.*Value is isolated based on user scripts, but the current tabObj implementation is actually shared between user scripts, which means it can create conflicts and surprises.

Tab.id Optional
integer. The tab's ID. Tab IDs are unique within a browser session.

@quoid I believe the session in browser session and sessionStorage have different meanings and ways of working, since browser session could actually persisted across browser closings and openings (depends on user's browser settings), but sessionStorage will be cleared every time browser closes.

I think we should do away the APIs in its current form, and just provide a privileged extensions API bridge similar to the getTabID or getTab(infos), with a clear warning to the user that this can be used to track tabs across origins.

We should also reference APIs such as sessions.setTabValue() and sessions.getTabValue() based on usage scenarios. (Currently not available in Safari)

What do you think? Please let me know what you guys think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants