68542 - hang when chosing edit|delete on a corrupted bookmark

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect, P1)

Description

[Fabian Guisset]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win95; en-US; 0.8) Gecko/20010210
BuildID:    2001021020

Ok this is an edge case, but easy to reproduce.
It is dependant on bug 68537 and bug 68539.
This is a very bad hang, it even brings down X (so says bzbarsky).

Reproducible: Always
Steps to Reproduce:
1) Launch Mozilla 
2) Bookmarks|Bookmarks Manager (BM) 
3) Resize the BM to have a vertical scrollbar (i.e. you don't see the last
bookmarks) 
4) Select your root bookmark directory ("Bookmarks for ") 
5) Hit "New Folder" on the toolbar 
At this point you have a new folder but it is at the bottom of the list and the
window hasn't scrolled on its own. 
(bug 68537).
6) Scroll down to the new folder using the vertical scrollbar
7) The inline edit cell has lost focus. Click on it to get the focus back on it
8) Type some name
9) Hit enter or click outside the cell
The name change is not saved. So far this is bug 68539
10) Click again on that new folder whose name hasn't been changed to edit it inline
Result : A "coupled" folder with name "Bookmarks for <profilename>" will be
created under the New Folder. You cannot select one without the other. 
11) Chose Edit|Delete

Actual Results:  Hangs

Expected Results:  Doesn't create a "coupled" bookmark and/or doesn't hang

Happens on all platform

Comment 1

[Boris Zbarsky [:bzbarsky]]

A comment.  What this actually did was make nothing be able to take focus but
the menu in bookmarks.  So it could be a windowmanager issue.  But the Mozilla
hang was definitely there.

Comment 2

[Boris Zbarsky [:bzbarsky]]

severity critical -- this is a crasher bug essentially

Comment 3

[Boris Zbarsky [:bzbarsky]]

After a bit of investigation, looks like the X hang happens because Mozilla
hangs with a menu open.  Using the "Del" key instead of the "Delete" menu option
gets around this.

Looks like an infinite loop in the JS engine to me... attaching a stack trace of
where Mozilla is when I stop it once it's hung.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: where we seem to be when we hang

Comment 5

[Brendan Eich [:brendan]]

Infinite loops in JS are policed by the DOMBranchCallback (dom/src/base
nsJSEnvironment.cpp).  If you've waited for 4M backward branches (which could
take a while -- be patient), and the callback hasn't put up a confirm dialog
asking whether you want to stop the script, then something else must be going wrong.

/be

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

I believe I have a fix of sorts for this problem.

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: some bookmarks goodness

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

OK, there are several bugfixes in this patch, namely:

1) a hack for this bug. There seems to be a template builder problem here, and 
I'll file a bug on waterson later.
2) Kill the 'create in' disclosure button on the add bookmarks dialog.
3) Place some more shared stuff in bookmarksOverlay.xul
4) Prevent dataloss when dragging a bookmark onto an aggregated folder by 
preventing the drop.
5) First (easy) half of springloaded folders. (**)
6) Bookmarks sidebar panel, based on the new code. 

Notes: 
(*) The diff shows me accidentally undoing blake's event.button change. Fixed in 
my tree.
(**) This should probably go into the same piece of code that handles tree drag 
feedback, rather than being hard-coded into this tree, but it'll do for now. 

Need r=, a=. 

Comment 9

[Fabian Guisset]

*** Bug 70836 has been marked as a duplicate of this bug. ***

Comment 10

[Paul Chen]

Marking nsbeta1+, p1, mozilla0.9.1

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Fixes checked in. 

Comment 12

[Claudius Gayle]

I marking this verified but mostly out of attrition. All of the other bugs cited in the description
are still evident but I'm no longer able to produce a hang, or the 'coupled fodlers' or anything
like that. There is already another bug about delete probelm so I see no need to do anything
else but mark this bug VERIFIED Fixed.