369560 - ###!!! ASSERTION: *** XBL doc with no root element! Something went horribly wrong! ***: 'Error', file m:/trunk/mozilla/content/xbl/src/nsXBLService.cpp, line 417

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Alex Vincent [:WeirdAl]]

Attachment: stack trace

I hit this assertion visiting the URL above.  Firefox 1.9a2 (frozen code, not off the release), Windows.

Comment 1

[Alex Vincent [:WeirdAl]]

How interesting.  At nsXBLStreamListener::Load(nsIDOMEvent * aEvent=0x04bb4738)  Line 417 + 0x1c bytes	C++, we have the following for documentURI->mSpec:

+		mData	0x080e5bc8 "http://static.technorati.com/css/moz-text-overflow.xml"	char *

The XML file referenced is a 404.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This assertion is bogus.  There are all sorts of reasons an XML document could have no root element.  I think we should just remove the assert.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Agreed, though we should keep it as a warning since this usually shouldn't happen.

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1

per comment 3, NS_ERROR -> NS_WARNING

Comment 5

[Nickolay_Ponomarev]

mozilla/content/xbl/src/nsXBLService.cpp  1.22

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Why was this marked in-testsuite-?

Comment 7

[Nickolay_Ponomarev]

Because I thought that *absence* of bogus assertion is not something worth testing.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Testing that invalid docs don't assert seem like a good thing to me.

Comment 9

[Nickolay_Ponomarev]

OK then, I'll ask sayrer if there's a way to test for assertions yet. BTW XBL code has lots of NS_ERRORS for malformed content, I hit three when making a testcase.

Comment 10

[Nickolay_Ponomarev]

For the reference, I talked to sayrer a while ago and he said the test harnesses don't currently support testing for assertions and it's not a top priority.

I filed bug 370767 for that, but we might want to just check a simple testcase in and hope that developers will run the test suite in debug builds with assertions not suppressed :)

Comment 11

[Nickolay_Ponomarev]

Attachment: content/xbl with the test added

Here's the test I had in my tree, sorry it's not in the patch format - CVS sucks for making it hard to make patches that add new directories. If anyone thinks it should be checked in, feel free to r+ :)

Comment 12

[Daniel Holbert [:dholbert]]

FWIW, we trigger this warning when running layout/style mochitests, e.g. in this tbpl log:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=10222026&tree=Firefox

{
11722 INFO TEST-PASS | /tests/layout/style/test/test_ch_ex_no_infloops.html | Setting 'text-rendering' to '2ch' should not cause infinite loop - auto should not equal
WARNING: *** XBL doc with no root element! Something went horribly wrong! ***: file ../../../../content/xbl/src/nsXBLService.cpp, line 405
11723 INFO TEST-END | /tests/layout/style/test/test_ch_ex_no_infloops.html | finished in 1434ms
}

It frightened me a little when I first noticed it locally (thought I might've broken something), due to the asterisks and the "horribly wrong". Given comment 2 & comment 3 here, we could probably stand to soften the language a bit. :)

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: followup

Here's a followup to make this less scary sounding.

Basically just s/Something went horribly wrong/this usually shouldn't happen/ based on comment 3.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 607787 [details] [diff] [review]
followup

r=me

Comment 15

[Daniel Holbert [:dholbert]]

(Sorry, the DestroyFrom chunk in this patch is clearly unrelated -- I'll remove that before landing)

Comment 16

[Daniel Holbert [:dholbert]]

Landed followup to soften the warning:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/5b3f94379f5c

Comment 17

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/5b3f94379f5c