Rapptz / power-tabs

Vertical tab groups for Firefox
https://addons.mozilla.org/en-US/firefox/addon/power-tabs/
MIT License
25 stars 6 forks source link

Opened tabs remembered but all is put to the same group. It doesn't remembers which tab is in which group. #11

Open trancephorm opened 6 years ago

trancephorm commented 6 years ago

Anyone having the same problem on FF 58.0.1?

Rapptz commented 6 years ago

I can't reproduce this.

Under the hood the extension associates a groupID a tab is with using the browser.sessions.setTabValue API and then retrieves it later.

Could you give more info on this?

trancephorm commented 6 years ago

Updated to FF 58.0.2, but it's even worse, it doesn't remember opened tabs, even if FF is set to remember opened tabs. I would give you more info on this, but I don't know where to get that info, please help. This addon is crucial to me, and it's pretty good, I just don't see how can I make it functional.

trancephorm commented 6 years ago

Left only Power Tabs addon enabled and it behaves the same :(

trancephorm commented 6 years ago

I'm on Linux 64-bit.

trancephorm commented 6 years ago

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sessions/setTabValue (not compatible with FF58, so that' it :(

Rapptz commented 6 years ago

That API does work on anything after Firefrox 57.

Could you give me some more information on what you're doing to make it forget tabs? Such as closing the windows and what not?

trancephorm commented 6 years ago

Sorry I got it wrong from that URL. Doing nothing special, just closing FF and re-opening it makes it forget tabs every time. As I said, I disabled all the addons except Power Tabs and it's still the same. Previously, on the same OS I had FF 56 with Tab Groups addon which worked flawlessly. I made about 7 groups, color them differently, and since it's FF58.0.2 all tabs are visible all the time, I understand I gotta wait for FF59 to hide tabs from inactive groups which is what I want.

More, I don't really understand why your addon has only 57 installations? As far as I see, it's the only decent tab manager out there for FF57+, addon so fundamental I don think it should be natively supported by browser.... World is weird. ;)

Rapptz commented 6 years ago

Could you expand on "makes it forget tabs"?

Are you sure those tabs are assigned to different groups and if so how did you do the assignment? There are different ways:

  1. Right click a tab and go to Move to Group and assign a tab there.
  2. Use the sidebar and then drag and drop tabs from one group to another.
  3. Right click a tab (or multiple) from the sidebar and select Move To.

Apologies if you've already done this, I just want to rule this out in particular.

As for why it only has 57 installations, I don't really advertise this add-on anywhere except one post I did on reddit. I made this for a couple of friends and myself who wanted a tab groups replacement for Firefox 57+.

trancephorm commented 6 years ago

Used sidebar and dragged and dropped tabs from one group to another, that's how I did it. Yes, tabs were listed under the different groups... Everything functions great until I close the browser and then when I opet it, there are no tabs anymore :( ... I mean, ok, it's only one installation on less important machine of mine, but before I install it on other ones, I need to make sure this won't happen.

Rapptz commented 6 years ago

If this is a legitimate bug on my end (and not on Firefox's) I do want to figure out why it happens. Thanks a lot for your patience.

I do have a rather strange and convoluted request if you could regale me a bit.

Could you do the following steps for me?

  1. Go to about:debugging
  2. Tick "Enable add-on debugging"
  3. Go down to Power Tabs entry
  4. Click Debug
  5. There'll be a popup asking you to confirm if you want to do this, press Confirm.
  6. In the Console tab enter this code: (async function() { let tabs = await browser.tabs.query({currentWindow: true}); for(let tab of tabs) { let groupId = await browser.sessions.getTabValue(tab.id, "group-id"); if(!groupId) console.log(`bad tab ${tab.id}`); } console.log('done'); })();

Let me know if you see any bad tabs.

trancephorm commented 6 years ago

I'm getting this reply: Promise { : "pending" } done

Rapptz commented 6 years ago

That means that the tabs do actually have their session storage properly set.

I'm wondering if you have some kind of setting that makes all the session storage invalidated on window close but I doubt it since you said that tabs get restored.

Only other thing I can do to rule this out as my issue is the following snippet:

(async function() {
    let tabs = await browser.tabs.query({
        currentWindow: true
    });
    let groups = {};
    for (let tab of tabs) {
        let groupId = await browser.sessions.getTabValue(tab.id, "group-id");
        if (!groupId) {
            console.log(`bad tab ${tab.id}`);
        }
        else {
            if(groups.hasOwnProperty(groupId)) {
                groups[groupId].push(tab.id);
            }
            else {
                groups[groupId] = [tab.id];
            }
        }
    }
    Object.entries(groups).forEach(([groupId, t]) => console.log(`${groupId} -> ${t.length} tabs`));
})();

That should print the groupId associated with the tabs and how many tabs they have. Just to make sure they're not all assigned to one group when they shouldn't be.

trancephorm commented 6 years ago

output:

Promise { : "pending" } e2fddf45-8b02-43a7-a787-f9b3b7b53fb5 -> 5 tabs

trancephorm commented 6 years ago

There's another variant of behavour: It remembers tabs, but it puts them all into the same group. This addon is so crucial to me I really want to help any way I can, please post what should I do next.

Rapptz commented 6 years ago

I don't see anything weird there unless they're supposed to be split between multiple groups at the time you ran the script.

trancephorm commented 6 years ago

Yes, they were all in the same group. I moved them to various groups and then I get: Promise { : "pending" } e2fddf45-8b02-43a7-a787-f9b3b7b53fb5 -> 5 tabs e9c1f485-20d6-411d-b268-80cee8ee1b58 -> 2 tabs 6654db53-84be-44e4-a6fa-16af71cfca66 -> 2 tabs b2ba9b5c-ef47-44a5-9636-364671e1a929 -> 3 tabs a5560544-0627-41e5-8f87-969f6ae00f04 -> 1 tabs onActivated: 3/4 -> {"tabId":4,"windowId":3} main.js:428:5 activeSync listen.js:401:3 onActivated: 3/12 -> {"tabId":12,"windowId":3} main.js:428:5 activeSync listen.js:401:3 onActivated: 3/4 -> {"tabId":4,"windowId":3} main.js:428:5 activeSync

So it seems everything is ok... Right now I'm going to close the browser and open it and will tell if they are still in the same groups...

trancephorm commented 6 years ago

They are all moved to the first group now. :(

Rapptz commented 6 years ago

Honestly this issue is very strange to me. I don't see anything wrong with the way I'm setting the session storage so that's clearly not the issue as per the script output. I just tried to reproduce it locally using 60a1 on Nightly (today's build) with no issue.

There must be something interfering with the session storage somehow when you close the window. Some sort of setting in about:config or privacy option is my guess.

trancephorm commented 6 years ago

Let me know if there's any about:config setting you need to know how it's set in my browser.

mac1202 commented 6 years ago

I also got this issue on firefox 60.0a1 on manjaro linux.

Boymann commented 6 years ago

I'm on 59.0b12 and get the same behaviour. Changes in my firefox are the tabhide enabled, and I amped content process limit to 7.

trancephorm commented 6 years ago

I need this fixed badly so I can start using newest FF. How much money do you need to fix this?

Rapptz commented 6 years ago

@trancephorm this isn't a money thing, it's more of a time thing on my end. Plus this is kind of hard to reproduce and fix since I don't do anything with the session storage that would cause them to clear. It also happens at inconsistent levels depending on the person reporting it.

ShinAreku commented 6 years ago

I have run into this problem as well. It is frustrating as the addon is so good!

I had the issue on firefox 60.0 on both Windows and Linux. After removing some other session management addons and reloading all tabs in another window it resolved on the Windows system but the Linux system is remaining the same.

I have tried with a fresh install and profile and it will still put all tabs into the currently selected group. The above debug checks all come out the same as well with tabs correctly being assigned to groups.

Promise { <state>: "pending" }
9be52bff-f468-4cff-a2e3-0f2f6142e906 -> 4 tabs 
ce2cc762-f55c-4b78-837e-371c0e5f709f -> 10 tabs

But then reverting to all tabs in the current group after a restart.

Promise { <state>: "pending" }
ce2cc762-f55c-4b78-837e-371c0e5f709f -> 14 tabs

However, I noticed that if you disable power tabs, then restart firefox and enable it again then it works correctly and all tabs are grouped from when you disable it. I tried this with safe mode so that it disables all addons and it acts correctly again upon a normal startup. This means the issue is occurring on shutdown and not startup and so definitely points to your idea about the storage being affected on window close.

Is there any way to confirm if perhaps the tabIDs are no longer matching to the groups and so they get defaulted to the current group? I have noticed that sometimes tabs will be labelled 'New Tab' but have the previous open tab contents - as if it had created the new tab in an empty group and then the session restore started bringing in tabs. There is a mention of how for the getTabValue function that a "restored tab does not get the same ID as the original" but I am not sure if that would be to blame for this.

trancephorm commented 6 years ago

Thanks a lot for taking a closer look. It's absolutely crucial & fundamental add-on without which I just cannot use any browser. I'll give you a 100 USD if you solve it, not kidding.

It's a total mind maze to me why there is still not a good solution for multitabbed browsing in >FF56

ShinAreku commented 6 years ago

@Rapptz Could you please advise on how I could help you debug this further. In particular on how the tabs are matched to the groupID on disk. There definitely is a bug in how the groupIDs are saved in the session though I cannot see how it is occurring.

I can see the storage.js file with the groupIDs being updated each time a change is made. The sessionstore.jsonlz4 stores all the normal session data but with power tabs includes group-id entries to associate the tabs. On the problem systems it seems like a groupID that does not exist gets used.

My storage.js: {"name":"Main","uuid":"20420bf4-bdaf-4984-b649-603bfdb4ddff","open":true,"active":true,"colour":"#000000"},{"name":"Puppet","uuid":"8bdf9d46-0346-4f18-944b-777588fef69f","open":true,"active":false,"colour":"#000000"}

First entry in the sessionstore.jsonlz4: "extData":{"extension:power-tabs@rapptz-addons.com:group-id":"\"20420bf4-bdaf-4984-b649-603bfdb4ddff\""}

All other group-id entries in the sessionstore.jsonlz4: "extData":{"extension:power-tabs@rapptz-addons.com:group-id":"\"ebb80123-afe5-40fe-8bd0-9fa8f62e9a23\""}

The ebb80123 group-id seems to come out of nowhere so it seems that a false groupId is being returned somewhere. Creating other groups does not seem to affect anything and the ebb80123 remains the same between restarts. The first group-id will change depending on which group is currently active. This happens constantly on the Linux system but seems to fix itself on Windows systems after the first few restarts.

ShinAreku commented 6 years ago

Thanks to the jsonlz4 files showing up weirdly in plain text the previous post was partly incorrect.

I have checked over the session files with the sessionstore properly uncompressed to json and can confirm that all the tabs are correctly being assigned their group-id in the extData for the current window.

The ebb80123 group-id is actually coming from previously closed windows where all the tabs are assigned this group-id. I tested on the working system and it seems to correctly save the group-id for closed windows but will then lose the order of tabs as well if multiple windows are opened and closed before closing the main window. Opening a new window, closing the main one with all your tabs, restoring it and then closing down also seems to cause it.

So this seems like related issues - incorrect group-id being assigned to closed windows in the session store and multiple windows causing tabs to lose their group.

I have also seen the issue where all other tabs are closed which seems to be caused by dragging the current tab in the sidebar but most of the time it will just slow down for a few seconds, but that seems like a separate problem.

Celti commented 6 years ago

It looks like the issue here is not with the sessionstore being mangled on shutdown, but rather it's getting ignored on restore.

The fundamental problem appears to be that prepare() in background/listen.js is declared as an async function, meaning all of the listeners are added at more-or-less the same time as it starts. The resulting race condition means prepare() is fighting with onTabCreated() for whether each restored tab keeps its previous group ID or gets the active one — losing means all the tabs wind up in the active group when Firefox is restarted.

I have submitted PR #21, which should fix this (and #16).

Celti commented 6 years ago

Ugh, I take that back, that breaks opening new tabs within groups at the same time as it fixes the original issue.

Celti commented 6 years ago

Okay, so even accounting for the fact that I completely forgot await doesn't work outside of an async function, there is still something weird going on here — if I force prepare() to finish before the assortment of addListener() calls, new tabs fail to allocate to any group, and I'm really not sure why.

Beyond that, however, the grouping failure on start is definitely a race condition of some sort — if I simply wrap everything from ensureDefaultSettings() onward in a 2000ms timeout, everything works flawlessly (at least on my laptop). 1500ms is a bit whiff, 1000ms fails about half the time.

Unfortunately, Firefox doesn't have any convenient event to, e.g., wait for the browser startup to finish before trying to handle tabs. I tried working around this by waiting for tabs.onUpdated events to stop firing for the length of a timeout (tried between 500 and 5000ms), but again ran into the issue of new tabs failing to be grouped.

I am closing #21 for now; anyone have any other ideas on fixing this?

Celti commented 6 years ago

Okay, this appears to be fixable without any code changes, by setting extensions.webextensions.background-delayed-startup to true in about:config (at least on latest Developer Edition).

I can reliably reproduce the issue with that option set to false, not while true.

desrod commented 6 years ago

I can reliably reproduce the issue with that option set to false, not while true.

I can confirm that this issue is not fixed in Quantum 60 on macOS and Linux, using either 'true' or 'false' with the above configuration option. It's still a bug, and every time the browser is closed, all tabs get moved into the group that had focus when the browser was closed.

For my session with over 100 tabs in 20 groups, this is an enormous pain to reconstruct every time I start the browser. It's easier to use folders in the standard Bookmarks, than rebuild this every time I launch it.