255027 - History sidebar context menu can contain wrong menu items, some of which cause a crash [@ nsAssignmentSet::GetAssignmentFor ]

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[PikeUK]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040809 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040809 Firefox/0.9.1+

If you right-click on a blank part of the history sidebar when no entries are
selected a menu appears that contains entries for both folders and individual
entries. Clicking Expand or Collapse from that menu crashes the browser.

Reproducible: Always
Steps to Reproduce:
1. Open the history sidebar (making sure not to select any history entries)
2. Right-click below the existing items (again making sure not to select any
entries)

Actual Results:  
A context menu containing three default (bolded) items (Expand, Collapse & Open)
appears, clicking on Expand or Collapse crashes the browser.

Expected Results:  
No menu should have appeared.

The equivalent behaviour in the Bookmarks sidebar is to show a very small empty
menu.

Comment 1

[PikeUK]

Incident ID: TB526275K

Comment 2

[Adam Hauner]

TB526275K:
nsAssignmentSet::GetAssignmentFor 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/xul/templates/src/nsRuleNetwork.cpp,
line 597]
nsTemplateMatch::GetAssignmentFor 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/xul/templates/src/nsTemplateMatch.cpp,
line 51]
nsXULTreeBuilder::GetResourceFor 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/xul/templates/src/nsXULTreeBuilder.cpp,
line 1533]
nsXULTreeBuilder::ToggleOpenState 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/xul/templates/src/nsXULTreeBuilder.cpp,
line 888]
XPTC_InvokeByIndex 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2028]
XPC_WN_CallMethod 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1287]
js_Invoke 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 941]
js_Interpret 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 2973]
js_Invoke 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 958]
js_InternalInvoke 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c,
line 1035]
JS_CallFunctionValue 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/js/src/jsapi.c, line 3673]
nsJSContext::CallEventHandler 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1297]
nsJSEventListener::HandleEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1434]
nsEventListenerManager::HandleEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1512]
nsXULElement::HandleDOMEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2841]
PresShell::HandleDOMEventWithTarget 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6086]
nsMenuFrame::Execute 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsMenuFrame.cpp,
line 1670]
nsMenuFrame::HandleEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsMenuFrame.cpp,
line 453]
PresShell::HandleEventInternal 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6050]
PresShell::HandleEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5899]
nsViewManager::HandleEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/view/src/nsViewManager.cpp,
line 2285]
nsViewManager::DispatchEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/view/src/nsViewManager.cpp,
line 2025]
HandleEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/view/src/nsView.cpp,
line 79]
nsWindow::DispatchEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1067]
nsWindow::DispatchMouseEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 5261]
ChildWindow::DispatchMouseEvent 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 5511]
nsWindow::WindowProc 
[d:/builds/tinderbox/firefox-1.0/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1349]
KERNEL32.DLL + 0x363b (0xbff7363b)
KERNEL32.DLL + 0x24407 (0xbff94407)
0x00c8877e

There are 12 incidents with same signature in Aviary and 1.7 builds:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsAssignmentSet%3A%3AGetAssignmentFor+
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsAssignmentSet%3A%3AGetAssignmentFor

Comment 3

[Blake Ross]

The right fix is to just not show a menu unless they right click on an actual
item. Would also be good to file a separate bug on the crasher, for the long term.

Comment 4

[Ryan Polk (Quark)]

Additional info.  If you do the right click in Bookmark's Sidebar, JS Console shows:
Error: aSelection has no properties
Source File: chrome://browser/content/bookmarks/bookmarks.js
Line: 162

If you do this in History's Sidebar, JS Console shows:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsITreeView.isContainer]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://global/content/nsTreeController.js :: isContainer :: line 59"  data: no]

Comment 5

[Ryan Polk (Quark)]

Attachment: menu patch

This patch removes all but 'Select All' from the menu, since that still serves
a purpose.

I can't reproduce the crash, which makes it hard to work on that part.

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 157553 [details] [diff] [review]
menu patch

not quite what we want here, discussion on IRC on how to better do this

Comment 7

[Ryan Polk (Quark)]

Attachment: v2, better if clauses

As discussed, not asking for review till I talk to mconnor about something
related.

Comment 8

[Mike Connor [:mconnor]]

patch obsoleted by patch in bug 217472, since we are going to a single-select
tree, and select all is gone, so we just won't show a context menu without a
selection.

however, the patch is pretty good if we do an about-face on this.

Comment 9

[Ryan Polk (Quark)]

The menu part of this is fixed on aviary via bug 217472, and I couldn't
reproduce the crasher when testing.  Can anyone else reproduce the crash?

Comment 10

[PikeUK]

(In reply to comment #9)
> The menu part of this is fixed on aviary via bug 217472, and I couldn't
> reproduce the crasher when testing.  Can anyone else reproduce the crash?

I can no longer reproduce the problem on:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040908
Firefox/0.10

Comment 11

[Mike Connor [:mconnor]]

will check in on trunk under 217472

Comment 12

[Tracy Walker [:tracy]]

verified fixed as seen on Windows 2004-10-07-06-0.9,  Mac 2004-10-07-05-0.9 and
linux 2004-10-07-09-0.9