Conflict with "Simple Tab Groups" : Tree structure flattened when opening new window

[Elaws]

Short description

Tree structure is flattened when a new Firefox window is opened, with "Simple Tab Groups" extension installed.

Steps to reproduce

1. Start Firefox with clean profile.
2. Install TST and "Simple Tab Groups" (versions indicated below).
3. With "Simple Tab Groups", create 4 tab groups and add shortcuts to switch between them (e.g. : Alt + & to load custom tab group n°1, ..., Alt + ' to load custom tab group n°4).
4. In each of these tab groups, create a simple tree structure with TST. For e.g. :

Folder A

• Subfolder 1
  -- URL i
  -- URL ii

Folder B

• Subfolder 1
  -- URL i
  --- URL ii
• Subfolder 2
  -- URL i

5. Open a new Firefox window and switch between tab groups defined in "Simple Tab Groups" using your shortcuts.

6. Close one of the two opened Firefox windows.

7. Switch between tab groups in the remaining Firefox window : one or all the trees created in TST may display the following message (it is possible that these messages are displayed at step 6) :

"X" tabs are opened in a time. Do you want them to be grouped in a tree?
-> Group tabs.
-> Keep tabs flat

Expected result

The tree structure should not be modified.

Actual result

No matter which answer you choose in step 7, one or all of your trees are now flattened :

Folder A
Subfolder 1
URL i
URL ii
Folder B
Subfolder 1
URL i
URL ii
Subfolder 2
URL i

TST + "Simple Tab Groups" is very useful but since opening a new Firefox window is a frequent use case, tree structures keep getting flattened.

Environment

• Platform (OS): Windows 10
• Version of Firefox: 74.0.1
• Version (or revision) of Tree Style Tab: 3.4.8
• Version of Simple Tab Groups : 4.4.2.5

[piroor]

Confirmed, but I think this is hard to be fixed only on TST side. I'm not familiar to the implementation how Simple Tab Groups switches groups across windows, but they addons look to break each other on this situation.

The most easiest way to make they compatible is: modify STG to get/set tree structure via TST's API. TST already has APIs for such a purpose but they were undocumented. So I've written the spec of get-tree-structure and set-tree-structure APIs:

• https://github.com/piroor/treestyletab/wiki/API-for-other-addons#take-a-tree-structure-snapshot-of-tabs
• https://github.com/piroor/treestyletab/wiki/API-for-other-addons#apply-a-tree-structure-to-tabs

But I cannot force it to the author of STG to use these APIs, because TST is not the only one tree addon, because there are more other similar tree addons, and the STG author needs to write compatibility codes for each...

[irvinm]

@Elaws I used Windows Sandbox, loaded Firefox and added TST and STG, created 4 groups, assigned a keyboard shortcut for each and was able to switch between each group of test tabs and didn't lose any of my tree hierarchy. I am wondering if it is possibly something else is interfering with your situation.

Have you you tried a new profile with just TST and STG to see if that works?

[Elaws]

@irvinm I'm not sure if you reproduced the steps : after creating a tree structure in each of your tab groups, did you open a new Firefox window (since STG requires "Restore Firefox previous session", all your groups and tree structures should remain in this new window), switched between tabs groups and closed one of the two opened windows (steps 5 to 7) ?

Please let me know if the bug does indeed happen in this situation !

[irvinm]

Oh, I missed that! I'll give it a go ...

[irvinm]

Ok, I see what you are saying now and I can reproduce. It appears that is really has to do with the situation where STG is creating a group in a new window for the first time. In this case, STG seems to simply store and restore the set of tabs without setting the parent (openerTabId) when loading a new group in a window for the first time.

This also explains why everything works fine in the initial window I saw (all the tabs had their relationships properly created in that window) and also why all of this would continue to work even if you restarted the browser (assuming "restore previous session" was enabled) if you were only dealing with one window.

It would appear to me that STG would need to make changes on their end to get this to work. Tab Session Manager (session manager) had a similar issue a long time ago and had to add support for openerTabId parameter in tabs.create() in order to preserve the tree hierarchy. If the addon developer doesn't care about relationships, they can just create the group without the parent information which seems to be the case here.

I don't think there is anything @piroor will be able to do for this situation.

[Elaws]

Thanks for your feedback @irvinm. Should have done it earlier but I will report this issue to STG's author.

[Drive4ik]

@piroor
Hi!
I already made a fix, but noticed one bug. When a new window opens and the tabs are moved there using browser.tabs.move. And after moving the browser.tabs.query call comes in and it returns incorrect data in the openerTabId key.
For example, there are tabs

immediately after moving browser.tabs.query is called
and the result is this:

after ~ 200ms the call to browser.tabs.query returns everything correctly

I made a delay of 200ms before the call browser.tabs.query and it worked.
So the question is: can you suggest a solution without a delay of 200ms? I cannot refuse to call browser.tabs.query after browser.tabs.move.

P.S.
I noticed another bug - if the TST sidebar is open, the movement of 100+ tabs into new window is occurs very, very slowly (in minutes). I don’t even know what it is connected with. Most likely this is another bug (perhaps we will discuss in another topic).

Thanks

[Drive4ik]

P.P.S.
successorTabId key also need to save?

[mtnjustme]

I've faced this issue as well, what I thought in my head is why not making the ability to hide/unhide and reload/unload with the ability to mark all tabs opened in a selected tree, in this addon? If it was the case then i'd just make trees for everything in the Tree Style Tab addon and remove the Simple Tab Groups addon.

[piroor]

@Drive4ik Wow! That's a good news!

So the question is: can you suggest a solution without a delay of 200ms?

I'm sorry I forgot details about how TST updates openerTabId. The wrong openerTabId of tabs in the second screenshot is produced byt TST at here:

treestyletab/webextensions/background/handle-tree-changes.js

Lines 195 to 203 in 244b0ff

	Tree.onDetached.addListener((tab, _detachInfo) => {
	if (tab.openerTabId &&
	configs.syncParentTabAndOpenerTab) {
	tab.openerTabId = tab.id;
	tab.$TST.updatedOpenerTabId = tab.openerTabId; // workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1409262
	browser.tabs.update(tab.id, { openerTabId: tab.id }) // set self id instead of null, because it requires any valid tab id...
	.catch(ApiTabs.createErrorHandler(ApiTabs.handleMissingTabError));
	}
	});

When tabs are moved across windows, the tree structure is 1) completely cleared (TST sets the tab's ID to its openerTabId) and 2) restructured after all tabs are completely moved (TST sets openerTabId based on restored tree structure). The second screenshot means you are now between steps 1 and 2. Due to the bug https://bugzilla.mozilla.org/show_bug.cgi?id=1409262 modified openerTabId is not notified to addons, so we cannot know the time: when the internal openerTabId of Firefox's native tab is really updated and becomes accessible via WebExtensions API. This is the reason why you currently need to wait for 200 msec (or more/less) to get the final openerTabId value.

Anyway, we need to keep it in our mind that openerTabId can be modified by other addons not only TST. Sadly the safest way is "wait for a while until all modifications by other addons finish", and we should try to accelerate the operation based on addon-specific API (for example: https://github.com/piroor/treestyletab/wiki/API-for-other-addons ) if they are available, for better user experience.

(And, the best way is fixing of the bug https://bugzilla.mozilla.org/show_bug.cgi?id=1409262 on Firefox side.)

I noticed another bug - if the TST sidebar is open, the movement of 100+ tabs into new window is occurs very, very slowly (in minutes).

I think this is a generic performance problem. Currently I can't imagine how to solve this problem. I think it possibly requires fundamental restructuring like https://addons.mozilla.org/firefox/addon/ftt/

[piroor]

why not making the ability to hide/unhide and reload/unload with the ability to mark all tabs opened in a selected tree, in this addon?

@mtnjustme This is due to the project policy of TST. TST is concentrated to provide very basic tree management features, and other useful features fundamentally unrelated to "tree of tabs" should be provided by other feature-oriented addons. TST is expected that it should work with other tab related addons seamlessly. If there is no existing addon for a required feature, I sometimes develop such a helper addon by my hand.

[EpsilonRose]

I already made a fix

I seem to still be having this issue, but I'm not sure if I missed something. Just to double check @Drive4ik, did you push your fix to your addon and/or did the other bug you mentioned prevent it from working?

Also, I wanted to clarify something I noticed with one of the other posts. It seems like the only factors that effect whether the trees in a group get flattened are the window the group was last opened in and whether or not that window is still opened.

So, if I have two windows and I create a test group in the first window, then switch that window to a different group and open the test group in the second window it'll keep its tree structure. If I then close the second window and reopen the group in the first one, it'll have a flattened tree structure, even though it was originally from the still open window and no changes have occurred. Similarly, changing the second window to a different group, before closing it, will not change how the test group gets handled.

Conversely, if I create a second test group in the second window, then open it in the first window, and then close the second window, the second test group's tree structure be just fine. This holds true even if I switch away from the second test group before closing the window.

[irvinm]

@Elaws is this still an outstanding issue? If not, can you close this item?

[Elaws]

@irvinm Sorry for this late reply. I just read the previous posts and am not sure if the issue should be solved by now ? On my side, the issue can still be reproduced.

@EpsilonRose Did you have some update from Drive4ik ?

[Drive4ik]

This fix will be released with STG 4.5.2+

[Drive4ik]

@piroor
I have noticed that users say that sometimes the recovery is not correct. Drive4ik/simple-tab-groups#601 (comment)

Here is my code that makes openerTabId key sync for new tabs (after unarchive a group for example)

https://github.com/Drive4ik/simple-tab-groups/blob/1569c49d8e39c873f87dccbc1967865e4b16cddd/addon/src/background.js#L95-L114

Can you take a look at it and advise how it can be improved / fixed? Unless, of course, for this you do not need to write TST Lite in the STG addon 😄
Thanks

[piroor]

Hmmm...

If the problem is caused from the fact "STG updates openerTabId after they are loaded but TST doesn't apply updated openerTabId", I think there are only two possible solutions:

• A) Add something new API to TST to notify "request to synchronize latest openerTabId information" and call it from STG. TST will fetches current status of tabs and updates tree of tabs passively.
• B) Add something new mechanism to poll openerTabId of each tab periodically by TST itself. TST will refreshes tree of tabs actively.

Both solutions have merits and demerits. "A" looks robust but it needs more changes to STG. "B" is completed in TST side but I'm afraid that it can eats CPU resource periodically.

[piroor]

I forgot to think about the third option:

• C) Add something new pub-sub API to STG. TST will use the API to register a listener to STG, and updates tree of tabs with a message notified by STG.

@Drive4ik How about this idea?

[Drive4ik]

If the problem is caused from the fact "STG updates openerTabId after they are loaded but TST doesn't apply updated openerTabId"

Here's what I thought, the "problem" of synchronization and determination of the parents of tabs happens on the TST side, because STG creates a tab, and only then updates the openerTabId key. And TST cannot track it properly because of this:

Due to the bug https://bugzilla.mozilla.org/show_bug.cgi?id=1409262 modified openerTabId is not notified to addons

That is, if STG creates tabs at once with the required openerTabId, then TST will work correctly?
The idea with api is good, but it seems to me that this is plan B, I will try to create tabs right away with the required openerTabId key, and if this does not work with TST, then only through the api TST and STG.
I will have to sort the tabs before opening (creating), assigning openerTabId to subsequent tabs, and then sorting (browser.tabs.move) them back to their original location in the array. I'm afraid that there may be problems with those who have the browser.tabs.insertAfterCurrent key set to true. Also, I do not yet know the behavior of TST when creating a large number of tabs based on openerTabId, and then moving (sorting) them according to their original position.
In general, I will try.

[Drive4ik]

@piroor Can you please check this bug on STG 4.5.5?

[piroor]

@Drive4ik Thanks! I've tried Nightly 84.0a1 + TST 3.5.34 + STG 4.5.5. Sadly I've confirmed that tree structure is lost after switching of groups.

On my environment, original tree is:

• A
  • A1
    • A11
    • A12
• B
  • B1
    • B11
      • B111
  • B2
    • B21

After they are moved to another window, they become:

• A
  • A1
    • A11
      • A12
• B
  • B1
    • B11
      • B111
        • B2
          • B21

After investigation, I've realized what happens.

1. STG moves A1 across windows.
2. When A1 is detached from the source window, both A11 and A12 are detached from A1.
3. Because there is no more parent, TST tries to keep tabs grouped, so TST attaches A12 to A11.
4. STG moves A11 across windows.
5. STG moves A12 across windows.
6. TST attaches A12 to A11 based on the tree structure at the step 3.

This problem is solved when I deactivate this handler:

treestyletab/webextensions/background/handle-removed-tabs.js

Line 288 in f453299

Tab.onDetached.addListener((tab, info = {}) => {

But this is required for a case when just a single tab is detached from an existing window. Hmm...

[Drive4ik]

@piroor This problem persists even without the STG addon. Try to select all the tabs from the tree that you described above using Shift + Click on tabs on topbar, and move all these tabs to another open window.
By the way, I do not use the onDetached handler at all, it only causes problems and confusion. When the tabs are moved by the user with Shift + Click on tabs, I apply to each tab the window group it moved to.
After all, these onDetached and onAttached events are called for each tab that moves, and there can be an unlimited number of them ...

[piroor]

The commit 88cc871 should fix the problem I commented above. But I still see another problem triggered with session restoration and I'm investigating it...

[piroor]

Finally I've understood that TST had serious problems around restoration of the tree structure for tabs moved across windows. TST unexpectedly tried to "restore" the tree structure saved to the session data when tabs are moved across windows every time, and it can break the current tree structure constructed after they were restored (automatically by STG or manually). This unexpected behavior is now suppressed with recent commits 26be464 and 1f47265.

[irvinm]

@Elaws have you tried things with the latest versions of each addon? Are we at a point where this is working and you could close out this issue?

[Elaws]

Hi @irvinm,

I can still reproduce the issue with the latest versions of each add-on : I have not tried yet on a blank Firefox profile, so if you can't reproduce it's perheaps due to options of my add-ons !

The problem is a bit different now though : the tree structure is not completely collapsed as before, but the order of the folders is mixed (and all the icons of the tree have disappeared, so you have to refresh it to get them back, as before).

If I had the following structure :

A
B
C
- c1
- c2
-- c21

D
- d1
- d2

I may get the following after :

c2
-- c21
D
- d1
- d2

A
B
C
-c1

For this to happen, as described in the original post :

1. Define at least 2 tabs groups, each with a given tree structure and a shortcut.

2. With a shortcut, open one of the tab groups (say n°1).

3. Open another window. Use shortcut to display any tab group you want (say n°2 or any other).

Sidenote : If you happen to use shortcut of tab group n°1 in window 2, focus will switch to window where the tab group is already open, that is window 1 : this is a feature you may or may not want (if you open another window, you may want to keep focus in that window), but it is another subject

4. Let's say you have tab group n°2 open in window 2 (and still tab group n°1 in window 1). Close Window 2.

5. Go back to Window 1. Open tab group n°2 : the tree structure of tab group n°2 will be completely mixed as described above.

[piroor]

@Elaws I couldn't reproduce this case. I prepare tree with the structure and move those tabs across windows with STG, then I got the original structure as expected, even if such moving of hidden tabs happens for pending tabs for me.

[Elaws]

@piroor I just tried with a new blank Firefox profile and can reproduce the bug : this time, the original tree structure is completely flattened, as described in the first post. Closing the second window is an important step to reproduce the bug, along with using keyboard shortcuts to switch between tab groups.

Platform (OS): Windows 10
Version of Firefox: 83.0
Version (or revision) of Tree Style Tab: 3.6.2
Version of Simple Tab Groups : 4.7

[piroor]

@Elaws Thanks, I've seen a problem with these steps:

1. Prepare two groups with tree: G1 and G2.
2. Prepare two windows: W1 and W2.
3. Set a group for each window: W1-G1, W2-G2.
4. Close the W2. (Then STG opens some tabs in the W1 and hides them immediately.)
5. Switch from the G1 to the G2 on the W1.
   • Expected result: G2 tabs are opened with tree.
   • Actual result: G2 tabs are opened as flat tabs, instead of mixed tree.

As described above my result is flat tabs instead of mixed tree. I've checked debug logs of TST. Logged events at the step 4 are:

1. Tabs in the W2 are removed (by me).
2. New tabs are opened in the W2 (by STG). They have no openedTabId for each.

Next I've tried collecting debug logs on STG. The log says that when "New tabs are opened in the W2 (by STG). They have no openedTabId for each" happens STG tried to restore tabs with openerTabId, but unexpectedly all restored tabs have their own id as their openerTabId instead of their tree parent. They look to be cleared by TST when "Tabs in the W2 are removed (by me)" happens...

[piroor]

I've found why openerTabId is unexpectedly cleared on my case. When G2 is moved from W1 to W2, TST restores tree structure at first. But after that TST internally clears openerTabId of moved tabs in G2 by the handler for tabs.onAttached (called when tabs are moved from W1 tot W2). The commit 26d9d24 prevents such unexpected clearing.

I've tried steps to reproduce with the latest TST containing the fix, and sadly I still see flattened tabs in G2 at W1 after W2 is closed. STG's debug log says that G2 tabs moved from W1 to W2 have their own id as their openerTabId unexpectedly at the log for tryRestoreMissedTabs. This is because each tab has invalid openerTabId when STG calls its own cache.setTab(). TST updates openerTabId of tabs after STG caches such invalid openerTabIds, and STG has no chance to know correct openerTabId updated by TST.

Safe steps to restore tabs with their structure including workarounds to update cached tab information by STG are:

1. Start Firefox. There is a window: W1.
2. Prepare two groups and tree of tabs for each group: G1 and G2.
3. Set G1 active at W1.
4. Open a new window: W2.
5. Set G2 active at W2.
   • STG moves G2 tabs from W1 to W2.
   • TST clears openerTabId of G2 tabs when they are detached from W1 for safety.
   • STG caches openerTabId of G2 tabs. At this time STG has invalid openerTabId for cached G2 tabs.
   • TST updates openerTabId of G2 tabs when they are attached to W2, after STG caches invalid openerTabId for G2 tabs.
6. Create a new group G3.
7. Set G3 active at W2, and switch back to G2.
   • At this time STG memorizes correct openerTabId for cached G2 tabs.
8. Close W2.
   • Tabs in G2 are moved to W1 by STG. TST successfully restores their tree structure.
9. Set G2 active at W1.
   • You'll see G2 tabs with correct structure.

[piroor]

Finally I've realized that openerTabId is cleared by Firefox itself even if TST doesn't clear it, when tabs are moved across windows. Thus now I'm trying to create a pull request for STG to keep the information: https://github.com/Drive4ik/simple-tab-groups/compare/master...piroor:keep-opener-tab-id-for-tabs-moved-across-windows?expand=1

[Elaws]

@Elaws Thanks, I've seen a problem with these steps:

1. Prepare two groups with tree: G1 and G2.

2. Prepare two windows: W1 and W2.

3. Set a group for each window: W1-G1, W2-G2.

4. Close the W2. (Then STG opens some tabs in the W1 and hides them immediately.)

5. Switch from the G1 to the G2 on the W1.
   
   * Expected result: G2 tabs are opened with tree.
   * Actual result: G2 tabs are opened as flat tabs, instead of mixed tree.

Yes, this is the problem I tried to describe in the original post, perheaps not precisely enough ! I thought you had already reproduced this problem.

[irvinm]

@Drive4ik have you looked at the pull request yet?

[irvinm]

@Elaws given there isn't anything more to do on the TST for now, we are waiting for the pull request to be evaluated. Unfortunately, there hasn't been any progress on that side and no check-ins since November. Could you close this item and when the pull request is incorporated (in some form) and if it still doesn't work, we could always reopen this issue.

[irvinm]

@piroor there haven't been any check-ins to "Simple Tab Groups" since November 2020. Maybe it would be best to close this item and if the pull request is ever merged or discussed we can discuss it here or in a related issue.

[Elaws]

Hello,

No news concerning the pull request to this date : is there a simple way to merge your code into the main of STG and build ourselves ?

Perheaps there are better solutions nowadays than TST/STG combo, but I've yet to investigate.

Thanks

[piroor]

@Elaws The author does some commits at May in this year, so the project looks not stopped yet. However, if you seriously need the change I did, you need to fork the STG repository, merge my change, and build it by yourself, then you'll load it as a temporary addon via about:debugging. If you hope to install it as a regular addon, you need to upload the forked version STG as a new private extension.

[Elaws]

Just a quick note to let you know that the problem is still occurring. The tree and tab group structure is regularly not restored properly on Firefox startup. In addition to that, when restored from a STG backup, trees are flattened and groups of tabs made in a tree aren't recovered.

I've not found yet a better combo than TST + STG (tried sidebery lately, but you have to manually restore previous session each time), so it'd be wonderful if tree and tab group structure was not so often disorganized.

I've seen some STG commits from @Drive4ik 4 days ago, so maybe some collaboration is possible if the problem is confirmed.

Thanks !

Workaround for now :

Manually restore a backup (auto_stg_backup .json files).