1422747 - `moveTabTo` doesn't do the right thing when there is only 1 tab in the window

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

[:Gijs (he/him)]

Trivial STR:

1. open new window
2. open browser console
3. run:

gBrowser.moveTabTo(gBrowser.selectedTab, 1)


Expected:

nothing happens

Actual:

new tab button is moved before the tab


You can see this with direct DOM manipulation by using:

gBrowser.selectedTab.parentNode.appendChild(gBrowser.selectedTab)

(or ... insertBefore(gBrowser.selectedTab, null) )

We can guard for this case in the "moveTabTo" implementation, but it would be better to fix the underlying bug, unless fixing the underlying XBL bug is too difficult to bother doing before we get rid of XBL entirely.

Boris, do you have any idea what's going on here or why the fixes from bug 472020 are insufficient here and/or whether it's worth fixing something here over adding a guard to moveTabTo() ?

Comment 1

[:Gijs (he/him)]

(This is currently exposed through webextensions, but we're adding a workaround in bug 1422211)

Comment 2

[Boris Zbarsky [:bzbarsky]]

> or why the fixes from bug 472020 are insufficient here

Those fixes are known to not fix all issues like this.  That's why some of the reftests that bug added are marked as failing....

So it's possible that this situation looks like https://searchfox.org/mozilla-central/source/layout/reftests/bugs/472020-1b.xul or https://searchfox.org/mozilla-central/source/layout/reftests/bugs/472020-2.xul ... or something else along those lines.

Emilio has poked at things like this recently; it's even possible that his in-flight patches fix this.  If not, we'd need to sit down and debug what's really going on here with the insertion points.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

I could take a look when I have some free time, whatever that means... :)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1422747: Make 2-level nested insertion points work correctly.

Review commit: https://reviewboard.mozilla.org/r/207686/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207686/

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8936960 [details]
Bug 1422747: Make 2-level nested insertion points work correctly.

Err, this was supposed to be in bug 1425362. Anyway I don't think it's worth fixing. And to fix this particular case without redistributing the world we should keep track of all the nested insertion points, which we don't, and I don't think we should start doing...

Comment 6

[Brian Grinstead [:bgrins]]

Is this fixed now? Following STR in comment 0 I don't see anything happen.

Comment 7

[:Gijs (he/him)]

(In reply to Brian Grinstead [:bgrins] from comment #6)

Is this fixed now? Following STR in comment 0 I don't see anything happen.

If the STR no longer break things then yes, this is fixed - no longer using XBL for the tabbrowser-tabs would presumably have fixed this based on the earlier comments. \o/