403962 - [FIX]TabSidebar extension and tabbar drag and drop and reordering is broken

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[Peter van der Woude [:Peter6]]

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

repro:
Open FF
Install TabSidebar (http://www.oxymoronical.com/web/firefox/TabSidebar/development)
open a few tabs
try to drag and drop, focus a tabs that's 1/2 in view.

result:
broken or partially broken.

regressionrange:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&filetype=match&whotype=match&sortby=Date&hours=2&date=explicit&mindate=1195141620&maxdate=1195147439&cvsroot=%2Fcvsroot

bug 372323 , bug 400794 or bug 401569

Comment 1

[Boris Zbarsky [:bzbarsky]]

I'm not following the steps to reproduce here.  What should I be dragging?  Where?  Should the sidebar be open or closed?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Nevermind.  I think I see where the issue might be.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So this should be a regression from bug 372323.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Likely fix

Jonas, the issue here is that the tabstrip binding sorta looks like this:

  <content>
    <arrowscrollbox>
      <children/>
    </arrowscrollbox>
  </content>

and the arrowscrollbox node has its own binding and its own insertion point.  With the patch for bug 372323, we found the arrowscrollbox as the single insertion point for the tabstrip, then proceed to stick the new tabs (which are coming through ContentAppended()) in there instead of digging into the nested insertion point like we used to.

This patch just makes us look for a nested single insertion point (that is, makes sure that nesting all the way down through our insertion point chain none of them have bindings with a multiple insertion point).

The testcase is just a version of the bug 372323 testcase that has a binding with a single insertion point, but that node has a binding with multiple insertion points attached to it.  Without this patch, we get the disappearing-content issue of bug 372323 on this testcase.

Comment 5

[Boris Zbarsky [:bzbarsky]]

reed says this also fixes mousewheel scrolling in the Firefox tabbar (which is what I was debugging this with anyway, more or less).

Comment 6

[Damon Sicore (:damons)]

+'ing. P4.  

Comment 9

[Dão Gottwald [:dao]]

Tab drag&drop is also broken for the ordinary tab bar, so this broke multiple areas of our tabbing UI.

Comment 10

[Arie Paap [:wildmyron]]

Comment on attachment 288964 [details] [diff] [review]
Likely fix

Patch also fixes the drag&drop issues with the ordinary tab bar. Tested with local debug build in clean profile.

Comment 12

[Dietrich Ayala (:dietrich)]

Per the dupe (bug 404184), this also breaks keyboard navigation of tabs.

Comment 13

[dickvl]

Will this patch also solve the same issue as happens with drag&drop with "Undo Close Tab" that the tab reappears either at position 2 or at the end?

Comment 14

[Boris Zbarsky [:bzbarsky]]

Probably, but if someone wants to test go for it!

Comment 15

[Jesse Ruderman]

The patch doesn't fix the tab-switching shortcuts (Cmd+Alt+Left, Cmd+Alt+Right) for me.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Hmm.  Ctrl+pgup/pgdown works for me on Linux with and without this patch...  Are we sure this bug is the issue with the Mac tab switching?

Comment 17

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

I reopened bug 404184, it appears to be unrelated (caused by another patch).

Comment 18

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 19

[Peter van der Woude [:Peter6]]

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

VERIFIED for TabSidebar/Tabbar as far as I can see