51903 - Assertion on switching pref categories [@ nsXULDocument::GetElementsByAttribute]

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Blake Ross]

Radha and I both see an assertion when switching categories in the pref window, 
as the new pane loads, in a debug build with MOZ_DISABLE_JAR_PACKING on.

Comment 1

[John Morrison]

This is the assertion (from an email thread that preceded this bug).

###!!! ASSERTION: no doc root: 'root != nsnull', file
d:\seamonkey\mozilla\rdf\content\src\nsXULDocument.cpp, line 2606

Comment 2

[Blake Ross]

gah, I forgot to post the stack trace. sorry.

KERNEL32! bff768a0()
nsDebug::Assertion(0x015b8cb8, 0x015b8ca8, 0x015b8c78, 2606) line 256 + 13 bytes
nsXULDocument::GetElementsByAttribute(0x03617c9c, {...}, {...}, 0x0079928c) 
line 2606 + 32 bytes
XULDocumentGetElementsByAttribute(0x012b54d0, 0x02d36808, 2, 0x02d8de0c, 
0x0079933c) line 304 + 33 bytes
js_Invoke(0x012b54d0, 2, 0) line 731 + 23 bytes
js_Interpret(0x012b54d0, 0x00799cc4) line 2538 + 15 bytes
js_Invoke(0x012b54d0, 1, 2) line 748 + 13 bytes
js_InternalInvoke(0x012b54d0, 0x02d375f8, 47127584, 0, 1, 0x00799e58, 
0x00799de8) line 821 + 19 bytes
JS_CallFunctionValue(0x012b54d0, 0x02d375f8, 47127584, 1, 0x00799e58, 
0x00799de8) line 3175 + 31 bytes
nsJSContext::CallEventHandler(0x012b5980, 0x02d375f8, 0x02cf1c20, 1, 
0x00799e58, 0x00799e54, 0) line 906 + 33 bytes
nsJSEventListener::HandleEvent(0x034bcc64) line 154 + 64 bytes
nsEventListenerManager::HandleEventSubType(0x0340c660, 0x034bcc64, 0x013213a8, 
8, 7) line 788 + 19 bytes
nsEventListenerManager::HandleEvent(0x012d9630, 0x0079a6c4, 0x0079a654, 
0x013213a8, 7, 0x0079a6e8) line 1289 + 39 bytes
nsXULElement::HandleDOMEvent(0x013213a0, 0x012d9630, 0x0079a6c4, 0x0079a654, 1, 
0x0079a6e8) line 3306
nsXULTreeElement::FireOnSelectHandler(0x0349de1c) line 452
nsXULTreeElement::SetSuppressOnSelect(0x0349de18, 0) line 152
SetXULTreeElementProperty(0x012b54d0, 0x02d375f8, -5, 0x0079b01c) line 176 + 19 
bytes
js_Interpret(0x012b54d0, 0x0079b19c) line 2399 + 953 bytes
js_Invoke(0x012b54d0, 1, 2) line 748 + 13 bytes
js_InternalInvoke(0x012b54d0, 0x02cf1b98, 47409080, 0, 1, 0x0079b330, 
0x0079b2c0) line 821 + 19 bytes
JS_CallFunctionValue(0x012b54d0, 0x02cf1b98, 47409080, 1, 0x0079b330, 
0x0079b2c0) line 3175 + 31 bytes
nsJSContext::CallEventHandler(0x012b5980, 0x02cf1b98, 0x02d367b8, 1, 
0x0079b330, 0x0079b32c, 0) line 906 + 33 bytes
nsJSEventListener::HandleEvent(0x034b3904) line 154 + 64 bytes
nsXBLEventHandler::ExecuteHandler(0x034daf30, {...}, 0x034b3904) line 555
nsXBLEventHandler::MouseClick(0x034b3904) line 260 + 36 bytes
nsEventListenerManager::HandleEvent(0x012d9630, 0x0079ce30, 0x0079cd54, 
0x03406aa8, 2, 0x0079d128) line 861 + 23 bytes
nsXULElement::HandleDOMEvent(0x03406aa0, 0x012d9630, 0x0079ce30, 0x0079cd54, 2, 
0x0079d128) line 3306
nsXULElement::HandleDOMEvent(0x03405fa0, 0x012d9630, 0x0079ce30, 0x0079cd54, 2, 
0x0079d128) line 3323 + 39 bytes
nsXULElement::HandleDOMEvent(0x03405d80, 0x012d9630, 0x0079ce30, 0x0079cd54, 2, 
0x0079d128) line 3323 + 39 bytes
nsXULElement::HandleDOMEvent(0x03405870, 0x012d9630, 0x0079ce30, 0x0079cd54, 2, 
0x0079d128) line 3323 + 39 bytes
nsXULElement::HandleDOMEvent(0x034057f0, 0x012d9630, 0x0079ce30, 0x0079cd54, 2, 
0x0079d128) line 3323 + 39 bytes
nsXULElement::HandleDOMEvent(0x03405720, 0x012d9630, 0x0079ce30, 0x0079cd54, 1, 
0x0079d128) line 3323 + 39 bytes
PresShell::HandleEventInternal(0x0079ce30, 0x00000000, 0x0079d128) line 4042 + 
45 bytes
PresShell::HandleEventWithTarget(0x012daa40, 0x0079ce30, 0x02d96ea0, 
0x03405720, 0x0079d128) line 4023 + 18 bytes
nsEventStateManager::CheckForAndDispatchClick(0x034839e0, 0x012d9630, 
0x0079d238, 0x0079d128) line 1811 + 59 bytes
nsEventStateManager::PostHandleEvent(0x034839e8, 0x012d9630, 0x0079d238, 
0x02d96ea0, 0x0079d128, 0x0352ca70) line 892 + 28 bytes
PresShell::HandleEventInternal(0x0079d238, 0x0352ca70, 0x0079d128) line 4062 + 
43 bytes
PresShell::HandleEvent(0x012daa44, 0x0352ca70, 0x0079d238, 0x0079d128, 1, 1) 
line 3977 + 23 bytes
nsView::HandleEvent(0x0352ca70, 0x0079d238, 28, 0x0079d128, 1, 1) line 379
nsViewManager2::DispatchEvent(0x012d92e0, 0x0079d238, 0x0079d128) line 1429
HandleEvent(0x0079d238) line 68
nsWindow::DispatchEvent(0x0352b334, 0x0079d238, nsEventStatus_eIgnore) line 614 
+ 10 bytes
nsWindow::DispatchWindowEvent(0x0079d238) line 635
nsWindow::DispatchMouseEvent(301, 0x00000000) line 3811 + 21 bytes
ChildWindow::DispatchMouseEvent(301, 0x00000000) line 4021
nsWindow::ProcessMessage(514, 0, 6946920, 0x0079d5b4) line 2889 + 24 bytes
nsWindow::WindowProc(0x00000764, 514, 0, 6946920) line 883 + 27 bytes
KERNEL32! bff7363b()
KERNEL32! bff94407()
0079863a()

Comment 3

[Peter Trudelle]

nominating for dogfood.

Comment 4

[Jim Roskind]

Does this actually crash in a production build, or is this a bogus assertion?

Given that I *think* we can continue after an assertion in a debug build (re: 
easy work-around), and most folks spend very little time messing with prefs 
(little impact), I'm giving this a dogfood-minus rating.

Note that IF this were a crasher on the production build, or if someone were to 
say that the assertion is really valid, and we are doomed to crash RSN, then 
this would make it to dogfood (or certainly beta3 with a P1 level).

Comment 5

[John Morrison]

The dogfood nomination was based on the way it impedes debugging work, when 
the thing being debugged is a pref panel for a problem unrelated to the 
assertion, as both evaughan and pinkerton experienced today.

The assertion is not bogus, but there is no crash as the function effectively
bails out, returning an empty list. The downside is that GetElementsByAttribute
is not returning the correct result, which would bust nsWidgetStateManager.js,
and hence the whole pref management scheme. So, this is nsbeta3 material.

Comment 6

[John Morrison]

cc: ben, FYI. 

Comment 7

[Jim Roskind]

Based on the above comment, I changed the dogfood-minus to need info.

Please be very specific about the user impact of this bug.  What aspects of 
using and changing prefs are broken?  What are the user-visible issues?

Thanks,

Jim

Comment 8

[David Hyatt]

--> bryner, related to his selection changes

Comment 9

[Brian Ryner (not reading)]

I don't know the root cause of this, but by luck, an optimization to onclick in
my tree already seems to fix/work around this.

I don't see this assertion every time I switch a pane though, so please see if
this actually fixes it for you.

Comment 10

[Brian Ryner (not reading)]

Attachment: patch

Comment 11

[John Morrison]

To answer jar's question: I'm not entirely sure the extent of how this affects
the end user and the reliability of using prefs. However, something somewhere
is going wrong -- nsWidgetStateManager.js depends on reliable execution of
getElementsByAttribute. Therefore, I wouldn't put any faith in bugs reported 
for failures related to setting prefs in prefs dialog, until this bug is 
solved. (See bug 51855 for an example of how scrambled the JS logic is in 
current builds). 

Setting to nsbeta3+ so bryner can check in the fix/workaround (although I too 
am a little mystified how that patch fixes this problem -- hyatt? a clue?)

Comment 12

[Brian Ryner (not reading)]

john, can you verify that this patch fixes the problem for you?

Comment 13

[John Morrison]

Sorry, should have noted that before. Yes, that patch puts an end to the 
assertion in my win2k build (last updated ~9pm 09/10, i.e. last night)

Comment 14

[Brian Ryner (not reading)]

Checked in fix.

Comment 15

[John Morrison]

verified fixed in a current debug build on win2k and linux (only reported on
win32 as far as I know).