277523 - [FIX]appendChild to a hidden box from a binding crashes... [@ nsCSSFrameConstructor::ContentAppended]

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[Xavier]

If you append a DOM element to a binding that implements the <children> tag WITH
the attribute 'includes' and inside a hidden box, it crashes !
If the box is not hidden it's ok
A testcase is shown here :
http://www.c-est-simple.com/bugzilla/hidden/
select hidden.xul, it will crash your browser.
hidden.xml is the binding containing the hidden box and the children tag.

Regards
xavier

Comment 1

[Frank Wein [:mcsmurf]]

Stacktrace:
nsCSSFrameConstructor::ContentAppended(nsCSSFrameConstructor * const 0x05f42810,
nsPresContext * 0x061d4da8, nsIContent * 0x02b6aecc, int 0x00000001) line 8418 +
3 bytes
PresShell::ContentAppended(PresShell * const 0x05f42810, nsIDocument *
0x05fc8970, nsIContent * 0x061c82d8, int 0x00000001) line 5084
nsDocument::ContentAppended(nsDocument * const 0x05f42810, nsIContent *
0x061c82d8, int 0x00000001) line 2070
nsXULDocument::ContentAppended(nsXULDocument * const 0x05f42810, nsIContent *
0x00000002, int 0x00000001) line 1171 + 12 bytes
nsXULElement::InsertChildAt(nsXULElement * const 0x05f42810, nsIContent *
0x0626c758, unsigned int 0x00000001, int 0x00000001, int 0x00000001) line 1780
nsGenericElement::doInsertBefore(nsIContent * 0x00000001, nsIDOMNode *
0x0626c760, nsIDOMNode * 0x00000000, nsIDOMNode * * 0x0012f070) line 2904
nsXULElement::AppendChild(nsXULElement * const 0x061c82e0, nsIDOMNode *
0x0626c760, nsIDOMNode * * 0x0012f070) line 862 + 23 bytes
XPTC_InvokeByIndex(nsISupports * 0x061c82e0, unsigned int 0x00000012, unsigned
int 0x00000002, nsXPTCVariant * 0x0012f060) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
0x926b9768) line 2034 + 22 bytes
XPC_WN_CallMethod(JSContext * 0x0426c770, JSObject * 0x036b9768, unsigned int
0x00000001, long * 0x01ad26e8, long * 0x01ad2628) line 1287 + 10 bytes
js_Invoke(JSContext * 0x00000001, unsigned int 0x00000001, unsigned int
0x00000000) line 1293 + 17 bytes
js_Interpret(JSContext * 0x0426c770, unsigned char * 0x036b9998, long *
0x0012f540) line 3565
js_Invoke(JSContext * 0x00000001, unsigned int 0x00000001, unsigned int
0x00000002) line 1313 + 12 bytes
js_InternalInvoke(JSContext * 0x0426c798, JSObject * 0x05beb2c8, long
0x02a06e98, unsigned int 0x00000000, unsigned int 0x00000001, long * 0x0012f704,
long * 0x0012f714) line 1390 + 13 bytes
JS_CallFunctionValue(JSContext * 0x0426c770, JSObject * 0x05beb2c8, long
0x02a06e98, unsigned int 0x00000001, long * 0x0012f704, long * 0x0012f714) line
3804 + 26 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x05f42810, JSObject *
0x05beb2c8, JSObject * 0x02a06e98, unsigned int 0x00000001, long * 0x0012f704,
long * 0x0012f714) line 1352 + 24 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x0426c770, nsIDOMEvent
* 0x05c46740) line 184 + 74 bytes
nsEventListenerManager::HandleEventSubType(nsEventListenerManager * const
0x05f42810, nsListenerStruct * 0x06902d68, nsIDOMEvent * 0x05c46740,
nsIDOMEventTarget * 0x06f6154c, unsigned int 0x05c46748, unsigned int
0x00000007) line 1519 + 11 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x0167e718
sLoadEvents, nsPresContext * 0x00000000, nsEvent * 0x00000000, nsIDOMEvent * *
0x0012f8d0, nsIDOMEventTarget * 0x06f6154c, unsigned int 0x00000007,
nsEventStatus * 0x0012f994) line 1596
nsGlobalWindow::HandleDOMEvent(nsGlobalWindow * const 0x05f42810, nsPresContext
* 0x061d4da8, nsEvent * 0x0012f95c, nsIDOMEvent * * 0x0012f8d0, unsigned int
0x00000007, nsEventStatus * 0x0012f994) line 901
DocumentViewerImpl::LoadComplete(DocumentViewerImpl * const 0x00000000, unsigned
int 0x00000000) line 884 + 22 bytes
nsDocShell::EndPageLoad(nsDocShell * const 0x05f42810, nsIWebProgress *
0x05b270c4, nsIChannel * 0x06908f20, unsigned int 0x00000000) line 4372
nsWebShell::EndPageLoad(nsWebShell * const 0x05f42810, nsIWebProgress *
0x05b270c4, nsIChannel * 0x06908f20, unsigned int 0x00000000) line 674
nsDocShell::OnStateChange(nsDocShell * const 0x05b270c4, nsIWebProgress *
0x05b270c4, nsIRequest * 0x06908f20, unsigned int 0x06908f20, unsigned int
0x00000000) line 4298
nsDocLoader::FireOnStateChange(nsDocLoader * const 0x05f42810, nsIWebProgress *
0x05b270c4, nsIRequest * 0x06908f20, int 0x00020010, unsigned int 0x00000000)
line 1208 + 18 bytes
nsDocLoader::doStopDocumentLoad(nsDocLoader * const 0x05f42810, nsIRequest *
0x06908f20, unsigned int 0x00000000) line 832
nsDocLoader::DocLoaderIsEmpty(nsDocLoader * const 0x05f42810) line 729
nsDocLoader::OnStopRequest(nsDocLoader * const 0x05b270b4, nsIRequest *
0x041742d8, nsISupports * 0x00000000, unsigned int 0x00000000) line 653
nsLoadGroup::RemoveRequest(nsLoadGroup * const 0x00000000, nsIRequest *
0x05b270b4, nsISupports * 0x00000000, unsigned int 0x00000000) line 701 + 13 bytes

Comment 2

[Hermann Schwab]

wfm  Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20050108 Firefox/1.0+
crash Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20050107

Made some talkbacks, but the talkback server doesn´t accept today.

Comment 3

[Martijn Wargers (dead)]

Attachment: Testcase

The includes="span" in <xbl:children includes="span"/> in this testcase is
necessary to trigger the crash. so I guess you could call this an xbl issue.

Comment 4

[Martijn Wargers (dead)]

Doesn't crash for me in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113
But crashes for me in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040213
Firebird/0.8.0+

Comment 5

[Xanthor Sccas]

Crash also on GNU/Linux with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6)
Gecko/20050111

TB3146422Q

Comment 6

[Xavier]

I confirm, as said in the opening, that the 'includes' attribute is necessary to
get the crash.
However, using the binding in a xul file that just puts the specified tag
('span' in your test case, and 'label' in mine) as a child of the binding tag
won't crash (I'm mean without using JS).
It only does when the child element is created through DOM function and then
added to  the binding element.
So, could that be a DOM function bug ?

Regards.

(In reply to comment #3)
> Created an attachment (id=171682) [edit]
> Testcase
> 
> The includes="span" in <xbl:children includes="span"/> in this testcase is
> necessary to trigger the crash. so I guess you could call this an xbl issue.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: This is neither an XBL bug nor a DOM bug, folks....

We just need to skip content whose insertion point is display:none or otherwise
nonexistent -- such content shouldn't be showing anyway.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 9

[Xavier]

(In reply to comment #7)
I'm not sure I understand your fix. Do you mean the insertion won't be done on 
the targeted element if it is hidden ?!?

That would be a serious lack of feature, since the hidden attribute can be 
changed any time with expectancy to have all the content below it appear.

It would also be an inconsistency, because elements below an element that 
becomes hidden are not removed from the DOM...

And third, without the fix, everything worked fine if you ommitted 
the 'includes' attribute : the inserted elements are right were they belong, 
and show up when you unhide the parent element.

But as I say I may not have understood the fix.

Regards
Xavier Grosjean


> Created an attachment (id=171905) [edit]
> This is neither an XBL bug nor a DOM bug, folks....
> We just need to skip content whose insertion point is display:none or 
otherwise
> nonexistent -- such content shouldn't be showing anyway.

Comment 10

[Boris Zbarsky [:bzbarsky]]

First, no need to quote comments in their entirety.

Second, you're right --- you didn't understand the fix.  The code I'm changing
is the code that creates the rendering object tree.  Since the new node is being
inserted as a child of a display:none node as far as the rendering tree is
concerned, there should be no rendering objects created for it.  We were trying
to create one anyway, using the rendering object of the display:none node (null)
as the parent.  This crashed, of course.

None of this affects what the DOM looks like or what the rendering tree looks
like if you change the display property (the latter causes reconstruction of
rendering objects for the affected subtree).

Comment 11

[timeless]

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

Comment 12

[timeless]

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

Comment 13

[Bob Clary [:bc] (inactive)]

content/xbl/crashtests/277523-1.xhtml
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3