426805 - Duplicating All Bookmarks in Places causes temporary hang and an infinite recursive folder tree

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Ryan A. C.]

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

The Copy-drag seems to go pretty unchecked in Places, it allows copying of the All Bookmarks which is not normally allowed. Also of note, it allows copying of the 3 main sub-folders in the right-hand pane, but usually you can do that through copy-paste.

Reproducible: Always

Steps to Reproduce:
1. Ctrl-drag the All Bookmarks in Places
2. Drop it in Unfiled Bookmarks
Actual Results:  
Firefox will hang the first time you do this. If you kill the process and go back to places you can navigate the circular folders without problem, but attempting to drag the All Bookmarks again (with or without ctrl) will hang. Also, trying to copy the Unfiled bookmarks in the right pane after doing this will hang.

Expected Results:  
I don't think it should be possible to duplicate the All Bookmarks in the first place.

This is another way to hit Bug 424884, but this one causes a hang.
The tags folder doesn't allow dragging or copy-dragging, we should probably do the same to All Bookmarks.

Comment 1

[Ryan A. C.]

I attempted to repro this on Linux and it did hang for a while, but eventually gave an "unresponsive script" error:
Script: firefox/modules/utils.js:327

After stopping the script and trying to drag the all bookmarks the other hang was caused by:
Script: firefox/modules/utils.js:496

changing OS to all because it did hang on Linux for a while.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I can see the same after playing around. Here the STR:

1. Open context menu of All Bookmarks in left pane and select copy
2. Open Unfiled Bookmarks
3. Insert "All Bookmarks" within this folder

You can do step 3 multiple times. After a while Firefox hangs in the mentioned script (utils.js) above. DumpJSStack is too long to add the output here.

But anyway it shouldn't be possible to copy the "All Bookmarks" at all.

Dietrich, I can also *move* All Bookmarks to a subfolder. This makes the root folder disappear completely. Should I file a new bug for that issue or are you already aware of that?

Comment 3

[[:Aleksej]]

Note also that you cannot cut or delete All Bookmarks from the tree after copying it.

Comment 4

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

Didn't we say somewhere else that we were going to restrict copy operations on things like All Bookmarks and Unfiled bookmarks, and consider them to be special folders that couldn't be moved?

Comment 5

[Ryan A. C.]

Attachment: Disallow dragging of All Bookmarks

Simple enough fix for the root problem. Anything else to add to the list while we're doing so?

Comment 6

[Dietrich Ayala (:dietrich)]

(In reply to comment #4)
> Didn't we say somewhere else that we were going to restrict copy operations on
> things like All Bookmarks and Unfiled bookmarks, and consider them to be
> special folders that couldn't be moved?
> 

Yes, I think that we should not allow copy-ability of any special folders. However, both mconnor and mano voted this down. The patch in 423515 disallows dnd/cut moves, and provides functional copy support.

(In reply to comment #5)
> Created an attachment (id=315045) [details]
> Disallow dragging of All Bookmarks
> 
> Simple enough fix for the root problem. Anything else to add to the list while
> we're doing so?
> 

Thanks for the patch! I have a patch in bug 423515 that covers this a bit more thoroughly though. I'll cc you there.

Comment 7

[Ryan A. C.]

Comment on attachment 315045 [details] [diff] [review]
Disallow dragging of All Bookmarks

Ooops, that one breaks all dragging :P
Should have tested more than dragging All Bookmarks I guess.

Comment 8

[Dietrich Ayala (:dietrich)]

fixed in bug 423515. that patch makes it so that a true copy of the "All Bookmarks" folder happens on drop, instead of copying a shortcut to that folder.

Comment 9

[Tracy Walker [:tracy]]

verified with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042806 Minefield/3.0pre

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just one remaining question. Why the user has the opportunity to copy "All Bookmarks" or the sub folders at all?

Comment 11

[Tracy Walker [:tracy]]

I believe the main reason is that there has been sufficient interest from users to be able to access "All Bookmarks" from the Bookmarks Menu.  This makes that possible.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sounds reasonable. But the drawback is that it is only a snapshot of all the bookmarks from the moment the copy action was invoked. No idea if this makes much sense when users have to do it again and again to stay updated.

Comment 14

[Charles Twardy]

The recursive-copy hang EASILY happens in the Firefox 3 sidebar.  When trying to rearrange my bookmarks in the sidebar, I accidentally dropped "Bookmarks Menu" into itself.  The program hung with lots of hard drive activity, presumably as it recursed indefinitely.   I killed the program.

On restart, everything was fine though there was a deeply nested folder.  I deleted the first recursion.  After about 30s of hard drive scrubbing it was fine.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Charles, this is already bug 432370.

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