255715 - [FIXr] Crash [@ nsXBLPrototypeBinding::GetImmediateChild ] when a xbl:binding has no id

Status: VERIFIED FIXED

(Core :: XBL, defect, P1)

Description

[Xanthor Sccas]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040207 Firefox/0.8
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040207 Firefox/0.8

When we link a web page to a XBL file with a binding element with no id, Mozilla
crashes.
A testcase is given by the URL : 
The XBL file contains : 
<?xml version="1.0"?>
<xbl:bindings xmlns:xbl="http://www.mozilla.org/xbl">
	<xbl:binding id="b" />
	<xbl:binding />
</xbl:bindings>

If we add an `id="b2"` to the last xbl:binding, there is no more crash.

And even if you add `xbl:id="b2"` the browser still crashes...

Reproducible: Always
Steps to Reproduce:
1. Open a page linked with a XBL file with a binding element without id attribute
Actual Results:  
Crash

Expected Results:  
No  crash

Verified with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040813
Firefox/0.9.1+,
Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.6) Gecko/20040207 Firefox/0.8,
and Mozilla 1.8a3 Linux

TalkBack : TB569654X

Comment 1

[Ian Graham]

I can reproduce this on Windows XP, with 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040803

Comment 2

[WD]

Confirming Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7)
Gecko/20040803 Firefox/0.9.3

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB569889Z

Comment 3

[Ian Graham]

verified in today's Firefox nightly  --
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040815
Sorry, no talkback....  

Comment 4

[Boris Zbarsky [:bzbarsky]]

The problem is that nsXBLContentSink::ConstructBinding does nothing useful if
the ID is empty and then we try to call Initialize() on a proto binding we never
called Init() on and crash.

We should just report lack of an ID as an error and drop the binding.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Some one needs to attach a testcase to this bug so that QA can verify the fix
once it's fixed...  Web sites have a way of disappearing.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 158753 [details] [diff] [review]
Patch

Bryner, what do you think?

This patch will end up reporting misplaced elements if you actually put some
XBL stuff inside the binding with no id...

Comment 8

[Brian Ryner (not reading)]

Comment on attachment 158753 [details] [diff] [review]
Patch

Maybe we should explicitly log an error in this case so the author can find the
problem more quickly?

Comment 9

[Boris Zbarsky [:bzbarsky]]

We could, but it takes a lot of code (on the order of 50 lines) to log an error
to the JS console....  We really need a better API there.  If you want, I can
add such code here, I guess....

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 158753 [details] [diff] [review]
Patch

Not necessary to do it here, I guess, if it's a lot of code. It would be nice
to see the error reporting factored out so we can just say
LogError("noBindindID") and have that take care of looking up the string in the
properties file and logging it.

Comment 11

[Boris Zbarsky [:bzbarsky]]

That's bug 228205, mostly...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Though inside content/layout, bug 187007 will help a lot.  I filed bug 259861 as
a followup to add an error once that lands.

Checked in for 1.8a4.

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on Seamonkey trunk using Windows XP build 2004-09-26-05 and the
testcase of: http://pagesperso.laposte.net/taupe/xbl.xhtml

Comment 14

[Frank Wein [:mcsmurf]]

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