460666 - Drag & drop a bookmark to a specific place in a folder in Bookmarks Toolbar doubles it as last item

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[ourasi]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081019 Minefield/3.1b2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081019 Minefield/3.1b2pre

When trying to place a bookmark from location bar with drag & drop bookmark in a folder in Bookmarks Toolbar it will be placed in two places, the wanted place and last item.


Reproducible: Always

Steps to Reproduce:
1. Make a folder in Bookmarks Toolbar
2. Drag & drop URL from Location Bar and try to place it somewhere in the folder
3. 
Actual Results:  
Bookmark appears two times: in the wanted place and as last item in the folder

Expected Results:  
Should appear only in the wanted place

Comment 1

[Luke Iliffe (Harlequin99)]

Confirmed on XP - change platform?

Comment 2

[Ria Klaassen (not reading all bugmail)]

This is a regression from Bug 458233.

Comment 3

[Dietrich Ayala (:dietrich)]

Yes, this should be fixed on 3.1 -> blocking+.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

feels like the drop is served by both the toolbar and the submenus handlers, checkForMenuEvent has been removed in bug 458233, and the menu view is still using dragdrop, while the toolbar uses drop, i see the drop called by both dragdrop and drop events. Updating the menu view to the new api should solve the issue.

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

imho we could add an event.stopPropagation() in new dragstart, dragover and drop events in the menu view, they are already served and there should be no need to propagate down to the toolbar view... that would also make the dragstart parentNode check in toolbar.xml useless.

Comment 6

[Dietrich Ayala (:dietrich)]

mano, are you going to be able to work on this before freeze, or can i reassign it, or push it?

Comment 7

[Asaf Romano (gone)]

I'll try to get this done before the freeze.

Comment 8

[Dietrich Ayala (:dietrich)]

retargeting

Comment 9

[ourasi]

Now bookmark is placed to two places:

1. to wanted place in folder
2. as next item in Bookmarks Toolbar

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre - Build ID: 20081110212816

Comment 10

[Alice0775 White]

The following WORKAROUND patch works fine.

places\toolbar.xml
      <handler event="drop"><![CDATA[
+        //xxx bug 460666, To prevent duplicate bookmark when a link dropped on a folder.
+        event.stopPropagation();
+
+        //xxx bug 464242, To privent move wrong position when a bookmark dropped within own folder.
+        var target = event.originalTarget;
+        while(target){
+          if(target.localName == 'menupopup'){
+            return;
+          }
+          target = target.parentNode;
+        }
+
        // Cache the dataTransfer
        PlacesControllerDragHelper.currentDataTransfer = event.dataTransfer;

        var dropPoint = this._getDropPoint(event);
        if (!dropPoint)
          return;
        PlacesControllerDragHelper.onDrop(dropPoint.ip);
      ]]></handler>

Comment 11

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

original bug should be gone with tab detach fixes in bug 225680.

What is left is cleaning up the view to finally use only the new api.

Comment 12

[Luke Iliffe (Harlequin99)]

Confirming this appears fixed in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre ID:20081119072659

Comment 13

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i'm marking this fixed with bug 225680. If any part of places changes from that bug are going to be backed out (don't do that!) we will need to reopen this.

I'll file a separate bug for the view update

Comment 14

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

i filed bug 469703 for the view cleanup

Comment 15

[Tracy Walker [:tracy]]

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

Comment 16

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv