465693 - Dragging a bookmark from the bookmarks toolbar to tabstrip breaks bookmarks toolbar

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Luke Iliffe (Harlequin99)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081118 Minefield/3.1b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081118 Minefield/3.1b2pre ID:20081118224742

Dragging a bookmark from the bookmarks toolbar to the tabstrip to open a new tab seems to break most of the functionality of the bookmarks toolbar. Once the new tab is created the bookmarks toolbar is 'frozen' clicking bookmarks does nothing, the iBeam cursor from the drag operation is still showing, none of the live bookmarks or most visited tag folders will expand, bookmarks can't be dragged anywhere.


Reproducible: Always

Steps to Reproduce:
1.New profile
2.Drag a bookmark from the bookmarks toolbar onto the tab strip to open a new tab.
3.
Actual Results:  
Notice how the bookmarks toolbar is now 'frozen' clicking bookmarks does nothing, the iBeam cursor from the drag operation in step 2 is still showing, none of the live bookmarks or 'Most Visited' tag folders will expand, bookmarks can't be dragged anywhere.

Expected Results:  
Bookmarks in the bookmarks toolbar to behave normally

You can activate bookmarks and expand folders, but only if you have the mouse cursor right at the bottom a few pixels from the border between the bookmarks toolbar and the tabstrip.

Comment 1

[Luke Iliffe (Harlequin99)]

This appears to have regressed as a result of the checking to bug 225680 comment 122

Comment 2

[Alice0775 White]

When I drag BOOKMARK2 to b, the problem produces it.
When I dropped BOOKMARK2 to c via a, the problem does not occur.

[BOOKMARK1][BOOKMARK2]_______a____________________
[ tab ][ tab ] ___b__________c_________[+][ListAllTabs]

Comment 3

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

Setting to new requesting blocking.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Adding mano & mconnor - can you guys please look at this to see if it is indeed a regression from tear-off tabs?

Comment 5

[bomfog]

Should this be Places component, at least for find-ability, if not correctness?

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

(this is the right component, Places is being merged into this one)

So, I'm not seeing the issue as reported in comment 0 and comment 2, but I am seeing the following:

 - try to drag-to-reorder a bookmark on the bookmark toolbar
 - see symptoms like described (stuck I beam, bookmarks no longer work)

Can anyone else reproduce comment 0?

Comment 7

[Marco Bonardo [:mak]]

i can reproduce comment 6, not previous ones
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre

Comment 8

[Marco Bonardo [:mak]]

mh no, actually i can reproduce comment 0 too

Comment 9

[Dietrich Ayala (:dietrich)]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre

Cannot reproduce comment #0 nor comment #6 on Mac. I also tried many other scenarios, and cannot reproduce the problem.

Alice and Beltzner: What platform are you reproducing on?

Comment 10

[Luke Iliffe (Harlequin99)]

As noted in comment 0 it is possible to move a bookmark after a drag to the tabstrip, but only if you 'grab' it at the very bottom. If you then drag the bookmark to a new position in the bookmarks toolbar the problem seems to go away. The ibeam cursor disappears and bookmarks function correctly.

Comment 11

[Alice0775 White]

Comment #2 is on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre. And New profile.
See avi
Case 1 (works NG)http://space.geocities.jp/alice0775/_gl_images_/case1.avi
Case 2 (works OK)http://space.geocities.jp/alice0775/_gl_images_/case2.avi



And following patch was fixed the issue.
places\toolbar.xml

      <handler event="dragleave"><![CDATA[
+        this._dropIndicatorBar.removeAttribute('dragging');

        // Only handle dragleaves for the toolbar itself (and not for its child
        // nodes)

        if (event.target != this)
          return;

        PlacesControllerDragHelper.currentDropTarget = null;
        PlacesControllerDragHelper.currentDataTransfer = null;

        // Set timer to turn off indicator bar (if we turn it off
        // here, dragenter might be called immediately after, creating
        // flicker.)
        if (this._ibTimer)
          this._ibTimer.cancel();
        this._ibTimer = this._setTimer(10);

        // Close any folder being hovered over
        if (this._overFolder.node)
            this._overFolder.closeTimer = this._setTimer(this._overFolder.hoverTime);

        this._draggedNode = null;
      ]]></handler>

Comment 12

[Marco Bonardo [:mak]]

i suspect the issue is due to the negative margin used for the indicator bar, really the toolbar is not locked nor hang, if you try moving the mouse at the very bottom of a toolbar button everything works... it's like if the toolbar have been moved down the indicator bar without a negative margin. the "dragging" attribute on ib is never really removed because on leave we set a timer to remove it, but we then receive new dragover events that cancel the timer.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Dietrich: unless you disagree, I don't think this will block beta 2. If you think a safe patch can be prepared in the short term, find me on IRC to disagree.

Comment 14

[Marco Bonardo [:mak]]

we could simply comment out the

        if (event.target != this)
          return;

will be enough to restore previous behaviour (And code), even if it's not exactly correct, but was working like that before.

Comment 15

[Marco Bonardo [:mak]]

Attachment: patch

not perfect but good for b2 imo

Comment 16

[Marco Bonardo [:mak]]

Attachment: patch v1.1

small comment fix, we'll take this for the beta, and try to address real issue in a followup

Comment 17

[Dietrich Ayala (:dietrich)]

Comment on attachment 349292 [details] [diff] [review]
patch v1.1

talked over irc, looks good, r=me

Comment 18

[Marco Bonardo [:mak]]

Comment on attachment 349292 [details] [diff] [review]
patch v1.1

need explicit approval to land with closed tree.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 349292 [details] [diff] [review]
patch v1.1

a191b2=beltzner

Comment 20

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/62d26c7840a4

Comment 21

[Marco Bonardo [:mak]]

filed Bug 466073 as a followup to search for a more consistent fix for dragleave.

Comment 22

[Luke Iliffe (Harlequin99)]

Verifying this is fixed in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081120 Minefield/3.1b2pre ID:20081120222212