-
Notifications
You must be signed in to change notification settings - Fork 7
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
Ensure Landmarks runs correctly when navigating the history #202
Comments
Note: there are still problems with the workaround for content script non-reinjection on Firefox, so whilst the solution below seems good, it doesn't actually work yet; still getting "WebExtension context not found!" errors: <https://bugzilla.mozilla.org/show_bug.cgi?id=1390715#c6>. * Change the reconnection logic to use @lydell's method for coping with the background script not already being loaded on Firefox - https://bugzilla.mozilla.org/show_bug.cgi?id=1474727#c3 - thanks; I had been using an approach whereby it would try to reconnect n times in quick succession, but timing-based solutions are a bit yucky, whereas your method is elegant :-). Addresses #201. * At least temporarily reverse the changes in 6ea4e56 to make the code simpler. * Use @rpl's method for supporting the content script not being re-injected when navigating back to a previous page on Firefox - https://bugzilla.mozilla.org/show_bug.cgi?id=1390715#c4 - also thanks; I was really stuck with this and would certainly not arrived at the same conclusion and clean approach to the workaround :-). Ref #202. * Reinstate some of the background script logging from b766e49 to help with the content script re-injection/context errors ongoing investigation. * Add a FIXME to sort regarding DevTools being persistent across reloads on Chrome-like browsers.
I've tried this with both the in-devleopment 2.4.0 code and the released 2.3.1 code and get the same problem, so the cause is not down to the move from message-passing to port-based communications. It seems to be related to Firefox's "back-forward cache" ("bfcache") and I have added some info to some Bugzilla bugs, but not seen much movement on them, though there were some helpful suggestions from the bugs' reporters. Ideally I would have the time and expertise to really get to the bottom of it (thanks @graemecoleman for the suggestion of actually modifying ExtensionParent.jsm to add debugging info) and maybe file a new bug specifically for whatever the root problem is. However, at the moment there are other Landmarks features (and other projects) I'd like to work on (and would likely be better at), so will leave it for now. This only happens when going back in the history a certain number of steps, and can be mitigated by reloading (though Landmarks doesn't tell the user that), so it doesn't seem like a showstopper, though hopefully a solution will come to light soon. |
This seems to work fine on Chrome-like browsers (as others have noted in the bug report linked below). However, on Firefox, sometimes when going back/forward, the content script doesn't bootstrap again. This is because the page was only hidden, not destroyed and re-loaded, so the content script was not re-injected. Therefore the method for detecting that situation, proposed in the bug report, should be adopted.
The text was updated successfully, but these errors were encountered: