Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add GM.getTab and GM.saveTab #296

Merged
merged 9 commits into from Aug 6, 2022
Merged

add GM.getTab and GM.saveTab #296

merged 9 commits into from Aug 6, 2022

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

extension/Userscripts Extension/Resources/background.js Outdated Show resolved Hide resolved
@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
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.

None yet

2 participants