New tab via Ctrl-T when focusing non-last tab can have inconsistent insertion position

[lbmaian]

Short description

New tab via Ctrl-T when focusing non-last child tab can have inconsistent insertion position, specifically when the TST new blank tab setting is set to either "child of the current tab" or "next sibling of the current tab".

Steps to reproduce

1. Start Firefox with clean profile.
2. Install TST.

Case I

3. Set TST new blank tab setting to "child of the current tab"
4. Tree structure:

A
|- B (selected)
\- C

5. Create new tab via Ctrl-T
6. Close the new tab, select tab B again, and repeat from step 4 to start a new "iteration"

Case II

3. Set TST new blank tab setting to "next sibling of the current tab"
4. Tree structure:

A
|- B (selected)
\- C

5. Create new tab via Ctrl-T
6. Close the new tab, select tab B again, and repeat from step 4 to start a new "iteration"

Expected result

Case I

A
|- B
|   \- new_tab (selected)
\- C

Case II

A
|- B
|- new_tab (selected)
\- C

Actual result

Case I

For the first and every odd "iteration", the correct behavior:

A
|- B
|   \- new_tab (selected)
\- C

For the second and every even "iteration", always appears as an independent tab:

A
|- B
\- C
new_tab (selected)

Case II

For the first and every odd "iteration", the correct behavior:

A
|- B
|- new_tab (selected)
\- C

For the second and every even "iteration", always appears as an independent tab:

A
|- B
\- C
new_tab (selected)

Environment

• Platform (OS): Windows 10 Pro (64-bit), Version 1709, Build 16299.19
• Version of Firefox: Nightly 58.0a1 (2017-10-18) (64-bit)
• Version (or revision) of Tree Style Tab: 2.0.7

[piroor]

I think this is caused by the reason I wrote at #1038 (comment). If a new tab with loaded "about:newtab" page is opened and you try to open another new tab, then there is no "prepared about:newtab tab" and it is opened with the URI "about:blank". On the other hand, TST detects "new tabs opened by Ctrl-T" based on tab's URI. As the result, TST couldn't detect the secondary opened tab as a tab from Ctrl-T.

[piroor]

If my description at #1481 (comment) is true, you'll get expected result with enough delay, like: "Ctrl-T to open tab, close the new tab, wait 5 seconds, and Ctrl-T again."

[lbmaian]

I still reproduce this reliably on the latest nightly, even waiting 10 seconds between every step.

I've also reproduced this on the following test cases:

Case III

3. Set TST new blank tab setting to "child of the current tab"
4. Tree structure:

A (selected)
B

5. Create new tab via Ctrl-T
6. Close the new tab, select tab A again, and repeat from step 4 to start a new "iteration"

Expected result

A
\- new_tab (selected)
B

Actual result

For the first and every odd "iteration", the correct behavior:

A
\- new_tab (selected)
B

For the second and every even "iteration", always appears as an independent tab:

A
B
new_tab (selected)

Case IV

3. Set TST new blank tab setting to "next sibling of the current tab"
4. Tree structure:

A (selected)
B

5. Create new tab via Ctrl-T
6. Close the new tab, select tab A again, and repeat from step 4 to start a new "iteration"

Expected result

A
new_tab (selected)
B

Actual result

For the first and every odd "iteration", the correct behavior:

A
new_tab (selected)
B

For the second and every even "iteration", always appears as an independent tab:

A
B
new_tab (selected)

---

I've also found that similar behavior occurs when TST new blank tab setting is set to "a sibling of the current tab".

So, it's really when the "non-last tab" is focused before Ctrl-T, rather than "non-last child tab".

[lbmaian]

This is still reproducing reliably for me on the latest TST nightly on Firefox Developer Edition 57.0b14 x64.

I don't think this is a Firefox bug.

[sumyungguy]

I can reproduce this on FF Developer 58.0b1 on Mac. Also for example in case III, it's not necessary to close any tab. Just keep selecting tab A and making a new tab, it alternates between child and independent.

Waiting in between doesn't help. I also agree #1481 (comment) isn't correct, and it's not that Firefox bug.

[piroor]

Case III seems to be fixed for me by recent commits. On the other hand, on the Case IV, success case actually reports the new tab's url is about:newtab and failure case reports the new tab is about:blank...

[piroor]

After researching, I've realized that this event handler https://dxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#7198 does not executed on failure case. Because TST moves a newly opened tab from the end of the tab bar to next to the current, CSS transition of max-width is unexpectedly canceled and post process for newly opened tabs is blocked. Hmm...

[sumyungguy]

Case III also seems fixed for me with new TST nightly.

How do you see the tab's url? If I set "Guess a new tab as opened... about:blank", then in case III, I can create child tabs by pressing new tab key very quickly. But if I set that in case IV, next sibling tabs are not alternately created, so I think it must not be that it alternately gets "about:blank" as url.

[piroor]

Due to the reason I told at #1481 (comment) Firefox failed to prepare preloaded browser for about:newtab. By recent commits, now TST never move Firefox's tabs until their opening animation is finished, so I think most problems around new tab with about:newtab are fixed.

[piroor]

Required changes are very large 4b89bfd so I think there are some regressions.

[SXZ1]

@piroor you're right about regressions, I've just updated to 5569 and come across the fallout: order of tabs and tree structure are messed up again after session restore. I'll try to find new STR.

Also, as it turned out, the amount of time required to expand or collapse the stack depends on the number of tabs in a window. On my config it takes around 5 seconds to collapse/expand the stack with 5 child tabs in a window with 300 unloaded tabs. I'll try to find STR as well. It will be hard, though.

[SXZ1]

Filed a bug about fallout from this commit: #1513

[piroor]

OK, I've reverted the large commit and reintroduce small changes only for this issue. I think the commit should resolve this issue but there is another visual problem from slow operation.

[lbmaian]

Confirming this is fixed for me, thanks.