336286 - Dragging Local files to Bookmarks Toolbar no longer works

Status: VERIFIED FIXED

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

Description

[Mel Reyes]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060502 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060502 Minefield/3.0a1

When dragging local files (at least in windows) to the Bookmark Toolbar (at least) no longer creates a link.

Reproducible: Always

Steps to Reproduce:
1. Open Firefox
2. Drag any file, text, xpi, etc to the Bookmarks Toolbar

Actual Results:  
No bookmark added and the following error shows up in js console with strict & show enabled:

Error: data has no properties
Source file: chrome://browser/content/places/controller.js
Line: 1795


Expected Results:  
Bookmark to local file created.

Comment 1

[Peter van der Woude [:Peter6]]

any idea if this ever worked / regressiondate (estimate) ?

Comment 2

[Brett Wilson]

On 1.5 it seems to work on Windows but not Linux (at least mine, but it's old). I doubt it ever worked with places.

Comment 3

[Peter van der Woude [:Peter6]]

(In reply to comment #2)
> On 1.5 it seems to work on Windows but not Linux (at least mine, but it's old).
> I doubt it ever worked with places.
> 
I'm sure i've seen the comment about linux somewhere (couldn't find it)

Comment 4

[Mel Reyes]

Not really sure if it ever worked, I usually only does this for local page testing and just don't seem to remember it working since places was put in, sorry for the lack of details.

Comment 5

[O. Atsushi (Torisugari)]

I found that this is just because the first flavor is
"text/x-moz-url" (at least for PlacesController) and
the data does not contain any "\n". I guess the original
mime was "text/url-list", but gecko appended no linefeed
when converted it into "text/x-moz-url".

As a result, PlacesController.unwrapNodes(...) kicks the
droped data as it believes one-line "text/x-moz-url" is
malformed.

Comment 6

[O. Atsushi (Torisugari)]

Attachment: patch v.1

Just checked with Windows XP.
I'll take a look the behavior in linux, later.

Comment 7

[O. Atsushi (Torisugari)]

Attachment: patch v.2

Fixed incorrect increment handling in the previous patch.

Comment 8

[Niro]

The regression range on Windows XP is:
20060327-20060328

Comment 9

[Niro]

(In reply to comment #4)

> just don't seem to remember it working since places was put in,
> 

Places landed on trunk on 2006-03-02. Philor linked me to this page:
http://developer.mozilla.org/devnews/index.php/2006/03/02/places-enabled-by-default-on-trunk/
and it worked on 2006-03-27.

Meanwhile you can drag the file to the browser window and then to drag the url to the bookmark toolbar.

Comment 10

[O. Atsushi (Torisugari)]

(In reply to comment #8)
> The regression range on Windows XP is:
> 20060327-20060328

Yes, it's bug 323812, apparently, but it's not the point. What we
should decide now is "string without a line brake is a malformed
x-moz-url, or not".

If we regard it as *malformed*, probably we should fix
nsClipboard:FindURLFromLocalFile,
http://lxr.mozilla.org/mozilla/source/widget/src/windows/nsClipboard.cpp#756
because the data has no line break, and it's easy to append one here,
at least on Windows.

If no-line-break is OK, we should do some trick to make Firefox accept it,
something like attachment (id=229095). Or catching x-moz-file before
x-moz-url is also good to work around (in that case, it's still malformed
as a x-moz-url, so there can be side effects somewhere somehow).

I said "what we shoud decide now", and it's "what the owner/peers decide",
indeed. And I myself have no opinion on which is the better.

Comment 11

[Mike Connor [:mconnor]]

WFM Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070521 Minefield/3.0a5pre

Comment 12

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070523 Minefield/3.0a5pre ID:2007052307 [cairo]

I get 

Error: data has no properties
Source file: chrome://browser/content/places/utils.js
Line: 686

when I drag a local file (tried html and images) to the Bookmarks Toolbar

Reopening

Comment 13

[Dietrich Ayala (:dietrich)]

doesn't work on mac either

Comment 14

[O. Atsushi (Torisugari)]

I wonder why the assignee changes frequently. Can I steal this bug?

Comment 15

[Steve Won (stevewon on irc)]

Sure.It's set as a A6 blocker btw.

Comment 16

[Steve Won (stevewon on irc)]

Need to know ASAP if you are working on this or not.

Comment 17

[O. Atsushi (Torisugari)]

Attachment: Minimum patch v1

By "minimum", I mean it should work in the same way as Fx 2.0.x does.
And I expect it would ease reviewers.

(In reply to comment #16)
> Need to know ASAP if you are working on this or not.
I'm sorry for being late, I'm not that active on weekdays.

Comment 18

[O. Atsushi (Torisugari)]

Attachment: Minimum patch v2

I don't understand which is larger, apparently.

Comment 19

[Asaf Romano (gone)]

JS comments go in //, please. Agreed on the contents btw ;)

Comment 20

[O. Atsushi (Torisugari)]

Attachment: Patch

(In reply to comment #19)
> Agreed on the contents btw ;)

Maybe I should file another bug, but I'd ask you to review it too, anyway.
So I addressed that XXX comment.

Comment 21

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 22

[Asaf Romano (gone)]

Comment on attachment 272030 [details] [diff] [review]
Patch

I apologize for the very long delay here, this no longer applies :( If you don't have cycles to update this patch, let me know and I'll take care of that as soon as I can.

Comment 23

[Marco Bonardo [:mak]]

Attachment: attempted new patch

original patch does not apply anymore, i've tried to apply the same idea to the new code, and it's working.

Comment 24

[Asaf Romano (gone)]

Comment on attachment 295494 [details] [diff] [review]
attempted new patch

r=mano.

Comment 25

[Reed Loden [:reed]]

Checking in browser/components/places/content/utils.js;
/cvsroot/mozilla/browser/components/places/content/utils.js,v  <--  utils.js
new revision: 1.93; previous revision: 1.92
done

Comment 26

[Marcia Knous [:marcia]]

Tomcat and I aren't able to drag a local file to the toolbar using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008010704 Minefield/3.0b3pre. He sees the same issue on Leopard.

Comment 27

[Marco Bonardo [:mak]]

os x only problem?

this is working on windows, can someone test on linux?

Comment 28

[Martijn Wargers (dead)]

Marcia, please file a new bug for it not working on the Mac.

Comment 29

[Marcia Knous [:marcia]]

Filed Bug 411384 for le Mac.

Comment 30

[Marcia Knous [:marcia]]

Dragging locally to bookmarks toolbar works on Linux, as well as Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008010805 Minefield/3.0b3pre. Verifying fixed for those two. The actual bookmark name appears strange, but I can file a separate bug on that.

Comment 31

[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