475030 - can't drag tab to other place on tabbar

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090123 Minefield/3.2a1pre ID:20090123033224

repro:
select a tab
try to drag it a position left or right

result:
nada happens

Comment 1

[Asaf Romano (gone)]

WFM with a build that has this patch but might not include some other patches which landed today. I

Comment 3

[Jim Jeffery not reading bug-mail 1/2/11]

1. Pick one of your open tabs and attempt to move to the right or left one tab
position.
2. Note it does not move
3. Now take that same tab and move it past one tab either right or left, and it
now moves to the correct position, one tab to the right of where you wanted to
move it.

Comment 4

[Nochum Sossonko [:Natch]]

Nearly positive this was caused by bug 457651, specifically the second children tag.

Comment 5

[Boris Zbarsky [:bzbarsky]]

More likely caused by the comments.  See bug 474964 comment 7.

Comment 6

[IU]

For anyone having trouble reproducing this with a new profile, you initially need three or more tabs open.  With the three (unique) tabs open, attempt to drag and drop the first tab between the second and third tab.

Comment 7

[Alice0775 White]

I think this isuue is the CORE problem.
It is because the value of  gBrowser.mTabs[i].boxObject.screenX  is wrong. 

for (var i=0; i<gBrowser.mTabs.length; i++){
  alert(gBrowser.mTabs[i].boxObject.screenX);
}

A result does not increase sequentially.

Comment 8

[IU]

(In reply to comment #7)
> I think this isuue is the CORE problem.
> It is because the value of  gBrowser.mTabs[i].boxObject.screenX  is wrong. 
> 
> for (var i=0; i<gBrowser.mTabs.length; i++){
>   alert(gBrowser.mTabs[i].boxObject.screenX);
> }
> 
> A result does not increase sequentially.

shouldn't there be a semicolon after i++?

Comment 9

[Dave Garrett]

(In reply to comment #8)
> (In reply to comment #7)
> > for (var i=0; i<gBrowser.mTabs.length; i++){
>
> shouldn't there be a semicolon after i++?

No

Comment 10

[Alice0775 White]

Although a basis is scarce I think the following will fix the issue. 
Line 253 in scrollbox.xml
-var elements = this.hasChildNodes() ?
+var elements = (this.hasChildNodes() && this.childNodes.length > 1 ) ?

Comment 11

[Alice0775 White]

I am sorry, I wrote the wrong place. 
Please ignore Comment #10 .

Comment 12

[YUKI "Piro" Hiroshi]

I also think it is the bug of core components. When four tabs <A>, <B>, <C> and <D> open and I move <D> between <A> and <B>, then, DOM Inspector sais:
<xul:box>
  #comment ( This is a hack to circumvent bug 472020, otherwise ... )
  <xul:tab/> (A)
  <tab/> (D)
  <tab/> (B)
  <tab/> (C)
  #comment ( This is to ensure anything extensions put here will ... )
  <xul:toolbarbutton/>
</xul:box>
but actual layout is <D><A><B><C>.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

So was this fixed by the first patch in bug 474964 (landed 2009-01-23 16:48:05 PST)?

Comment 14

[YUKI "Piro" Hiroshi]

(In reply to comment #13)
> So was this fixed by the first patch in bug 474964 (landed 2009-01-23 16:48:05
> PST)?

No. I've merged the patch to my local Shiretoko, but it doesn't fix this bug.

Comment 15

[YUKI "Piro" Hiroshi]

I did some tests.

--------------------
gBrowser.removeAllTabsBut(gBrowser.selectedTab);
gBrowser.addTab('about:');
gBrowser.addTab('about:config');
gBrowser.addTab('about:buildconfig');
var tabs = gBrowser.mTabs;
var tab = gBrowser.mTabContainer.removeChild(tabs[2]);
tabs = gBrowser.mTabs;
gBrowser.mTabContainer.insertBefore(tab, tabs[1]);
--------------------
Then, the actual layout of tabs were broken. The inserted tab became leftmost tab unexpectedly.

--------------------
gBrowser.removeAllTabsBut(gBrowser.selectedTab);
gBrowser.addTab('about:');
gBrowser.addTab('about:config');
gBrowser.addTab('about:buildconfig');
var tabs = gBrowser.mTabs;
var tab = gBrowser.mTabContainer.removeChild(tabs[2]);
gBrowser.mTabContainer.appendChild(tab);
--------------------
Then, the actual layout was correct.

I think Gecko has a serious bug about layouting insertBefore()ed nodes.

Comment 16

[Peter van der Woude [:Peter6]]

(In reply to comment #13)
> So was this fixed by the first patch in bug 474964 (landed 2009-01-23 16:48:05
> PST)?

Yes, works with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090124 Minefield/3.2a1pre ID:20090124024054

Comment 18

[Dave Garrett]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090124 Shiretoko/3.1b3pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090124 Shiretoko/3.1b3pre

Yes, this is fixed in the latest nightly now.

Comment 19

[Dave Garrett]

I don't suppose some tests could be written for this?

Comment 21

[YUKI "Piro" Hiroshi]

Now both tests in comment #15 work correctly.
(Windows 2000, Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090124 Shiretoko/3.1b3pre)

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified on trunk and 1.9.1 with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090125
Shiretoko/3.1b3pre (.NET CLR 3.5.30729) ID:20090125052901

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre)
Gecko/20090126 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090126020313

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090125
Minefield/3.2a1pre ID:20090125034316

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090126 Minefield/3.2a1pre ID:20090126020316

It's already covered by Litmus:
https://litmus.mozilla.org/show_test.cgi?id=5973