398254 - Since the landing of 372769, I can no longer drag and drop tabs with TabMixPlus

Status: RESOLVED WORKSFORME

(Firefox :: Tabbed Browser, defect)

Description

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a9pre) Gecko/2007100103 Minefield/3.0a9pre ID:2007100103

1. New profile, start firefox
2. Install TabMixPlus extension from http://tmp.garyr.net/tab_mix_plus-dev-build.xpi (you may have to set extensions.checkUpdateSecurity and extensions.checkCompatibility to false in about:config)
3. Start firefox up, and open some tabs
4. Try and re-arrange the tabs on the tabbar.

Expected:
- You should be able to move tabs around, and view the tab drop icon.

Actual:
- You can't move tabs around. The mouse pointer takes on the 'denied' appearance (a circle with a line through)

Works fine:
- 20070928_0343_firefox-3.0a9pre.en-US.win32

Broken:
- 20070928_0655_firefox-3.0a9pre.en-US.win32 

Checkins to module PhoenixTinderbox between 2007-09-28 03:43 and 2007-09-28 06:54:
- http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1190976180&maxdate=1190987699

Due to bug 372769 ("Do lazy evaluation of XBL fields.") (Security-locked)

372769 has also caused bug 398135 and bug 397924, but maybe a fix for either of these bugs will fix this problem. Or maybe it is a TMP problem.

Comment 1

[Nikola Kocić]

Hmm... it works if you select "Built-in style" for Drop Indicator in TMP Options.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007100104 Minefield/3.0a9pre ID:2007100104

Comment 2

[Boris Zbarsky [:bzbarsky]]

So...  I can't reproduce this (on Linux).  I can move the tabs around just fine, though I get no drop indicator at all.  Should I be getting a little purple arrow indicator here like I do on branch?

Comment 3

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a9pre) Gecko/2007100205 Minefield/3.0a9pre ID:2007100205

Yes, you should get the pointy-down arrow when moving your tab. Strange you can still actually move them when me (on windows) is denied.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Hmm.  I do see a whole slew of JS console errors when I try to drag.  Could that be related?

In any case, should retest once the fix for bug 398135 goes in.

Comment 5

[AndrewM]

(In reply to comment #1)
> Hmm... it works if you select "Built-in style" for Drop Indicator in TMP
> Options.
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007100104
> Minefield/3.0a9pre ID:2007100104

Yes, it works for me as well if I change that option (on Windows this is), so I guess that's the workaround for the moment.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007100304 Minefield/3.0a9pre ID:2007100304

Comment 6

[Boris Zbarsky [:bzbarsky]]

Please retest in tomorrow's builds?

Comment 7

[(mostly gone) XtC4UaLL [:xtc4uall]]

still failing following the above mentioned STR using build
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102005 rv:1.8.1 ID:2007102005

Comment 8

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a9pre) Gecko/2007102001 Minefield/3.0a9pre ID:2007102001

Still fails for me, but the author of TMP has said he is able to fix it too, so maybe this bug should be INVALID and onemen can fix it in the extension code?

http://tmp.garyr.net/forum/viewtopic.php?p=21526#21526

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Still fails for me, but the author of TMP has said he is able to fix it too

Uh... someone actually has some information on _why_ this is failing?  Can we maybe by chance somehow perhaps get that information into this bug?  That would save me multiple hours of work trying to understand what this extension is doing and why it's failing.

Until we figure out what the problem is, we have no basis for judging whether it's invalid.  It could be our bug.  It could be the extension's bug.  In the former case, we need to fix it.  In the latter, the extension needs to fix it.

For now, we need to block on at least figuring out what's going on...

Comment 10

[tabmix.onemen]

for some reason TMP can not use gBrowser.mTabDropIndicatorBar after bug 372769 ("Do lazy evaluation of XBL fields.") landed.

when i add this to TMP startup code the everything work as before

gBrowser.mTabDropIndicatorBar = document.getAnonymousElementByAttribute(gBrowser, "class", "tab-drop-indicator-bar");

Comment 11

[Boris Zbarsky [:bzbarsky]]

I just traced through the code in TMP_miniT_init and we get a XUL element back when we ask for gBrowser.mTabDropIndicatorBar.

Are we getting back the wrong element?  Or am I looking at the wrong code?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Ah, I see.  We're getting back the wrong element.  The definition of the "mTabDropIndicatorBar" field is:

153       <field name="mTabDropIndicatorBar">
154         this.mTabBox.childNodes[0]
155       </field>

But there's code in tabmix.js that does:

  gBrowser.mTabBox.insertBefore(document.getElementById('vide-bar'),
                                gBrowser.mTabBox.firstChild);

where the 'vide-bar' thing comes from tabmix.xul.  If this code runs before the first time mTabDropIndicatorBar is accessed, then mTabDropIndicatorBar will end up as this 'vide-bar' node instead of the thing you want.

I think XBL itself is behaving as it should here; my only question is whether the tabbrowser binding should use a more robust method of getting mTabDropIndicatorBar.

Overto tabbrowser to decide on that.

Comment 13

[Mike Connor [:mconnor]]

Would be nice to make this more robust.  Since its lazy-init, we can use an anonid and get that, but TMP can work around their tweaking of the DOM if needed, so I don't consider this an absolute blocker.

Comment 14

[tabmix.onemen]

i can fix this in TMP.
i will make sure 
gBrowser.mTabBox.insertBefore(document.getElementById('vide-bar'),
                                gBrowser.mTabBox.firstChild);
will run only after first time mTabDropIndicatorBar is accessed.

Comment 15

[Dão Gottwald [:dao]]

mTabDropIndicatorBar is gone.