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

Improve handling of tabs suspended by a local installation of TGS #748

Closed
fwextensions opened this issue Sep 5, 2018 · 4 comments
Closed

Comments

@fwextensions
Copy link
Contributor

I added a comment on the most recent commit, but realized that's probably not very noticeable. So opening this issue as well.

While I'm seeing this issue loading when from master with existing suspended tabs (which is an edge case), I believe this bug would also apply to cases where the webstore version is trying to recover previously suspended tabs.

A related issue is that when trying to recover suspended tabs, queueTabScriptCheck() will try to recover any tab with suspended.html anywhere in the URL, even if it's not from a TGS extension. I opened a tab to a non-existent page on my site with that name and TGS triggered the exception when trying to recover it:

image

image

When calling isSuspendedUrl() to check for tabs that need recovering, I'd say that either strict matching should be used (which means that different installs of the extension wouldn't try to recover each other's suspended tabs), or the match should be on something like /chrome-extension:\/\/[^\/]+\/suspended\.html/, so that completely unrelated tabs that happen to include suspended.html somewhere don't match.

Specifications:

  • Extension version: 7.0.110
  • Browser & version: Chrome 68
  • Operating system & version: Win10
@deanoemcke
Copy link
Collaborator

@fwextensions sorry about this. it was a bug in some code i recently committed.
it should be fixed now.

as for the use of strict matching, i've also updated the code in this specific instance to use strict matching. however, i have not touched the many other places in the code that use loose matching.

perhaps the whole concept of loose matching should be removed from the extension, as it's really only to help users who use both the webstore version and a locally installed version.
i'm not sure this should be catered for in the production code. however, i do find it useful. it means that a call to something like 'sendRefreshToAllSuspendedTabs' will be able to update all suspended tabs, regardless of which version of the extension made the suspension.
it also helps out a lot if you want to load an old session from backup that has tabs suspended by a different version.

keen to hear your thoughts on this.

@fwextensions
Copy link
Contributor Author

My inclination would be to leave the suspended tabs from other versions alone by default, but have an option for a locally installed version to take over those tabs on demand. The extension could check if it's installed locally and then show that option if it is, maybe as a button on the Session Management page. Loading an old session could maybe just automatically load suspended tabs with the URL of whatever version is doing the loading.

At the least, the loose check should be tightened a bit to avoid the edge case of suspended.html appearing in non-TGS URLs.

@deanoemcke deanoemcke changed the title Installing from master throws exceptions when there are also tabs suspended by the webstore version Improve handling of tabs suspended by a local installation of TGS Sep 6, 2018
@deanoemcke
Copy link
Collaborator

I hope you don't mind, I updated the title to reflect this new direction of the thread.

@deanoemcke
Copy link
Collaborator

I've added a new debugging feature called 'tab claiming' which will allow you to revert any suspended tabs suspended by another version of the extension over to the current version of the extension (that is, the version running the debug page).

There is now also a handy link to the debug page in the about section of the extension options.

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

No branches or pull requests

2 participants