Closed Bug 76502 Opened 23 years ago Closed 23 years ago

Mozilla Freezes when trying to delete bookmarks in Bookmark Manager

Categories

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

Product:
SeaMonkey
Component:
Bookmarks & History
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.1

People

(Reporter: gerbilpower, Assigned: bugs)

References

Details

Attachments

(3 files)

Fix this, and a barrell of other bugs.
54.39 KB, patch
Details | Diff | Splinter Review
THIS IS NOT A REAL PATCH. The minimal change to fix the command problems.
4.31 KB, patch
Details | Diff | Splinter Review
minimaler NOT-patch
2.01 KB, patch
Details | Diff | Splinter Review 
Mozilla Freezes everytime when trying to delete bookmarks in Bookmark Manager
using the delete button on the keyboard.

Steps to Reproduce:
1. Launch Mozilla and open the Bookmarks Manager from the Bookmarks menu.
2. Try to delete a bookmark, or folder, with the delete key.

Expected Result:
The bookmark or folder gets deleted, and life goes on ...

Actual Result:
Mozilla freezes

Tested on:
Build 2001041804
Windows 98 SE
I cannot reproduce that bug using Win32 build 2001041704.
Build 2001041704 is yesterdays build, and I was able to delete bookmarks just
fine yesterday. The problem I just described is in build 2001041804.
oops. I'll look at this this evening.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
Reproduced with 2001041808/Linux.
OS: Windows 98 → All
bad-->worse. in the 4-19 builds the delete button and delete context menuitem do nothing,
while pressing the delete button continues to cause the app to hang. All/All
Hardware: PC → All
Whiteboard: critical to 0.9
This is likely caused by my renaming the edit commands and forgetting to rename 
a couple. I'm working on a fix right now. 
I've gotten this with Linux build 2001041908.  The stack trace for where
Mozilla seems to be stuck is:

#0  0x400db9d8 in nsAString::do_AppendFromReadable () at eval.c:41
#1  0x400dbe46 in nsAString::do_AppendFromElementPtrLength () at eval.c:41
#2  0x4008e862 in AtomImpl::ToString () at eval.c:41
#3  0x40c9e11d in nsXULAttributeValue::GetValue ()
   from mozilla/components/libgkcontent.so
#4  0x40c9ebb7 in nsXULAttribute::GetValue ()
   from mozilla/components/libgkcontent.so
#5  0x40ca62ef in nsXULElement::GetAttribute ()
   from mozilla/components/libgkcontent.so
#6  0x40ca6207 in nsXULElement::GetAttribute ()
   from mozilla/components/libgkcontent.so
#7  0x40cf2013 in nsXBLWindowHandler::WalkHandlersInternal ()
   from mozilla/components/libgkcontent.so
#8  0x40cf2a19 in nsXBLWindowKeyHandler::WalkHandlers ()
   from mozilla/components/libgkcontent.so
#9  0x40cf2aba in nsXBLWindowKeyHandler::KeyPress ()
   from mozilla/components/libgkcontent.so
#10 0x40bcf913 in nsEventListenerManager::HandleEvent ()
   from mozilla/components/libgkcontent.so
#11 0x40cba3ac in nsXULDocument::HandleDOMEvent ()
   from mozilla/components/libgkcontent.so
#12 0x40ca8530 in nsXULElement::HandleDOMEvent ()
   from mozilla/components/libgkcontent.so
#13 0x40ca8530 in nsXULElement::HandleDOMEvent ()
   from mozilla/components/libgkcontent.so
#14 0x40f10ec7 in PresShell::HandleEventInternal ()
   from mozilla/components/libgklayout.so
#15 0x40f10d5c in PresShell::HandleEvent ()
   from mozilla/components/libgklayout.so
#16 0x4103a62a in nsView::HandleEvent ()
   from mozilla/components/libgkview.so
#17 0x41043cae in nsViewManager::DispatchEvent ()
   from mozilla/components/libgkview.so
#18 0x41039fbd in HandleEvent ()
   from mozilla/components/libgkview.so
#19 0x40564cba in nsWidget::DispatchEvent ()
   from mozilla/components/libwidget_gtk.so
#20 0x40564be5 in nsWidget::DispatchWindowEvent ()
   from mozilla/components/libwidget_gtk.so
#21 0x40562f91 in nsWidget::OnInput ()
   from mozilla/components/libwidget_gtk.so
#22 0x4055f4b0 in handle_key_press_event ()
   from mozilla/components/libwidget_gtk.so
#23 0x4055f98f in dispatch_superwin_event ()
   from mozilla/components/libwidget_gtk.so
#24 0x4055f6fc in handle_gdk_event ()
   from mozilla/components/libwidget_gtk.so
#25 0x406f5d60 in gdk_event_free () from /usr/lib/libgdk-1.2.so.0
#26 0x4072aec1 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#27 0x4072b08c in g_main_run () from /usr/lib/libglib-1.2.so.0

I tried this with 2001-04-20-10, and pressing 'Del' doesn't freeze the browser
any more. BUT it doesn't delete the bookmark either. 'Delete' in the context
menu works just fine however.
Hm. Renaming the command fixes the button etc, but I still see the hang in my 
older tree. Updating to the tip again to see if I can verify what Andre is 
seeing. 
I see the exact same thing that André described on Win98SE with Build 2001042012.
Ben, I applied this patch locally and I can verify that it fixes this bug.
Build ID: 2001042113 on Linux.

Is there a way to split out the fix for only this bug, or do the changes
together make this bugfix possible?
Well there are several parts of the patch that fix the bug, and the broken 
clipboard ops (which is also a Bad Thing). I'm not sure exactly which now, but 
there are several, scattered here and there ;) 
ok, sr=alecf
Whiteboard: critical to 0.9 → critical to 0.9 have big patch sr=alecf need a=?
need to get a bunch of people to run with the patch to help
reduce risk of taking this big of a patch in 0.9...

we could also take this patch in 0.9.1 after the branch
and the pull to the branch if no one spots side effects.
Did you mean to comment this out? If so, how come? (Either just remove it
altogether, or add a comment above; e.g., ``we don't remember the focusElt here
because it would hurt the focus manager's poor little brain.'')

-      gBookmarksShell._focusElt = document.commandDispatcher.focusedElement;
+      //gBookmarksShell._focusElt = document.commandDispatcher.focusedElement;


Same here. Does the comment lie now?

       // We need to do this because somehow focus shifts and no commands 
       // operate any more. 
-      gBookmarksShell._focusElt.focus();
+      //gBookmarksShell._focusElt.focus();


And here:

+    nodeIsValidType: function (aNode)
+    {
+      switch (aNode.localName) {
+      case "button":
+      case "menubutton":
+      // case "menu":
+      // case "menuitem":

Looks ok otherwise, from the little I can understand.
I've had blake test the smallest patch (not really a patch, but he hand applied 
it) and he's verified that the key and the context menu item work (although 
strangely the button and edit menu item don't, although they work with the full 
patch). 

Good catch with the first commented out portion. The code in question should 
not be necessary, but a focus/controller bug means that it appears to be.

The other commented out section merely indicates future intent (to handle other 
node types when context menus on menus may be supported)
I'd rather take the smallest possible change here, as there are parts of the 
larger patch that are no longer desirable. If go is given for the third fix, I 
can apply this in my tree and check in.
adding nsbeta1+ P1.
Keywords: nsbeta1+
Priority: -- → P1
a=drivers when ready.
blake, you've applied and tested this?  Can you give it a r=?
waterson, can you sr (again) the smaller patch? thanks.
*** Bug 77406 has been marked as a duplicate of this bug. ***
ok, this looks fine to me.
I don't quite understand why we have to specifiy bookmark-specific versions of
each of these commands but for moz 0.9 this is ok. sr=alecf
Is it too late to land this on the branch? We still need a r= from someone,
right?
If we're still thinking about this one for 0.9, r=pchen
blake says this doesn't work.  Ben can you take another look at this please.
Here is the status of this in a CVS build from 2001-05-01:

o 'Del' on keyboard doesn't work. No freeze however.

o 'Edit -> Delete' doesn't work either.

o 'Delete' on the toolbar doesn't work.

o 'Delete' in the context menu DOES work.

To summarize: the context menu is currently the only way to remove bookmarks.
No messages are printed to the xterm during the other failed attempts by the way.
0.9 is wrapping up, pushing milestone out to 0.9.1 (please excues the trespass)
Whiteboard: critical to 0.9 have big patch sr=alecf need a=?
Target Milestone: mozilla0.9 → mozilla0.9.1
*** Bug 78586 has been marked as a duplicate of this bug. ***
a note to all:  since this bug seems to have morphed into more than its summary,
i would recommend checking out bug 72845, which has significant detail regarding
bookmark management errors.

Could we perhaps change the Summary of this bug to something like "Deleting
bookmarks in Bookmark Manager doesn't work"? That's more accurate (although it
works using the context menu).
The summary exactly describes the symptoms of the problem.
hong@netscape.com, no it doesn't. It doesn't freeze for me like it used to when
I try to delete a bookmark.
The summary doesn't seem to be correct. Mozilla doesn't freeze for me on a 
WinNT. The issue here is that the Delete key functionality doesn't work with 
Mozilla. This functionality should be added.
The bug, as filed is:

"Mozilla freezes every time when trying to delete bookmarks." (original 
reporter)

If the problem is now:

"The issue here is that the Delete key functionality doesn't work with 
Mozilla. This functionality should be added." (lynnw)

then a separate bug should be filed.  My point is not that your reported issue 
doesn't exist, but rather that it should be reported in another bug.  Changing 
the summary of one bug report to address multiple bugs is not the proper way to 
track an issue to resolution.  If the hang has been alleviated, this bug is 
FIXED and another bug exists.  
The Delete key functionality bug was filed as 78586, which someone marked as a 
duplicate of this bug. So, is 78586 a duplicate, or is this the same issue?
nav triage team:

Marking W4M because we no longer hang trying to delete, but delete doesn't work. 
That's bug 78586.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → WORKSFORME
verifying WFM.
Status: RESOLVED → VERIFIED
do not close this just yet, as I'm using it. It has a patch in it that fixes 
other bugs. If you want to mark fixed please relocate the patch elsewhere.
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
Fixed now.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Is this the cause of regression bug 42080?
there's no freeze anymore and I've verified bug 78586 to boot.
VERIFIED Fixed with 20010511 builds on all platforms
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: