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

tabTooltipNavButtons.uc.js doesnt work on unloaded/suspended tabs #48

Open
Ulf3000 opened this issue Jun 18, 2022 · 6 comments
Open

tabTooltipNavButtons.uc.js doesnt work on unloaded/suspended tabs #48

Ulf3000 opened this issue Jun 18, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@Ulf3000
Copy link

Ulf3000 commented Jun 18, 2022

you dont need to go through a linkedBrowser to get the sessionhistory of a tab.

you can just

let SH = SessionStore.getSessionHistory(this.triggerTab);

and then use SH.entries.length and SH.index in updateButtonState() and fillHistoryMenu()

like

let SH = SessionStore.getSessionHistory(this.triggerTab);

        if (SH.entries.length == 1 ) {
            updateSessionHistory(SH, true, true);
        } else if (SH.entries.length > 1 ) {
            updateSessionHistory(SH, true, false);

        }else{
            return false;
        }

then it also works on suspended tabs :)

i dont want to fork the whole repo , thats why i just write it in here ;)

@Ulf3000 Ulf3000 added the enhancement New feature or request label Jun 18, 2022
@aminomancer
Copy link
Owner

aminomancer commented Jun 19, 2022

Why would the script want session history of a pending tab? Session history is only used to fill the back/forward menu. You can't go back/forward in a browser that doesn't exist.

Edit: Also, we're using session history in parent on purpose. To take advantage of fission.sessionHistoryInParent if it's enabled. The code in your post above would risk unhandled exceptions because you're telling updateSessionHistory that SHIP is enabled (by passing true for the third argument) when it actually may not be (and probably isn't, since it's currently disabled by default)

@Ulf3000
Copy link
Author

Ulf3000 commented Jun 19, 2022

becasue when you restart firefox or load a session with session manager , all the tabs are unloaded and none of them have the back menu. And then its very inconvenient , you have to click, wait for tab to load and only then you can use the menu. you basically dont even know IF a tab has a sessionhistory unless you load it/activate it. time consuming and unintuitive imo

that code is just a quick fix , you can check later for parent and whatnot, but it seems to work fine here.

@aminomancer
Copy link
Owner

aminomancer commented Jun 19, 2022

becasue when you restart firefox or load a session with session manager , all the tabs are unloaded and none of them have the back menu. And then its very inconvenient , you have to click, wait for tab to load and only then you can use the menu. you basically dont even know IF a tab has a sessionhistory unless you load it/activate it. time consuming and unintuitive imo

That's not what I'm talking about. None of the unloaded tabs have the back menu because the back menu, like the back button, is for web navigation. That only exists when the tab has a browser, which is not the case for unloaded tabs. For any of this stuff to work, you have to load the tab.

that code is just a quick fix , you can check later for parent and whatnot, but it seems to work fine here.

Then try it and see for yourself. Unloaded tabs don't have browsers, therefore don't have docshells. There is no web navigation to go back in. The session history in the session store is just there to be restored when the browser loads. If you make the change you suggested, the menu won't do anything.

I mean, first it won't even open, because the buttons are disabled if there's no web navigation to go back/forward in. If you made them always enabled, the user would have no way of knowing whether the buttons are going to work or not. So that's effectively breaking the back/forward buttons, which are the main point of this script.

And then, even if you did that, the menu still protects itself from failure by checking for a browsing context before filling. If you deleted that check too, you'd just get a bunch of menuitems that don't do anything, because the menu calls tabNavButtons.gotoHistoryIndex which operates on the tab's browser. The whole purpose of the back/forward buttons and their context menu is web navigation, and it's just a fundamental fact about Firefox that there's no web navigation for pending tabs since their browsers were discarded and turned into serial tab states.

Edit: If you want web navigation to work in all the tabs after restoring, you can set browser.sessionstore.restore_on_demand to false in about:config

@Ulf3000
Copy link
Author

Ulf3000 commented Jun 19, 2022

i already did change the script for my purpose and its working fine. once you choose an entry from the back menu, a browser is created automatically and the tab gets activated. like i said i only changed those 10? lines of code.

@aminomancer
Copy link
Owner

If you changed the script, then paste it or submit a PR so I can see what you're referring to

@aminomancer
Copy link
Owner

Also, do your back/forward buttons work? Their left-click callbacks. Even if you skip all the checks, I'm pretty sure the automatic insertion (I'm guessing that's bug 1345098) doesn't work for webNavigation. I guess you could call gBrowser._insertBrowser(tab) beforehand if webNavigation is null. But I don't really like that because I don't know what might happen when you try navigating prematurely. If you look in the console you should see a warning when gotoHistoryIndex is invoked. Anyway, when I'm not so busy I'll consider testing an opt-in setting to force navigation on pending tabs. Thanks for the suggestion in any case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants