Last Comment Bug 330022 - CRASH using repeat + ul/li
: CRASH using repeat + ul/li
Status: RESOLVED FIXED
:
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: ---
Assigned To: aaronr
: Stephen Pride
:
Mentors:
Depends on: 306247
Blocks: 331209
  Show dependency treegraph
 
Reported: 2006-03-10 04:19 PST by Allan Beaufour
Modified: 2016-07-15 14:46 PDT (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Testcase (490 bytes, application/xhtml+xml)
2006-03-10 04:20 PST, Allan Beaufour
no flags Details
patch that fixes problem, but also traps on other testcases (1.52 KB, patch)
2006-03-10 11:51 PST, aaronr
no flags Details | Diff | Splinter Review
testcase crashes with new patch (2.07 KB, application/xhtml+xml)
2006-03-10 11:52 PST, aaronr
no flags Details

Description Allan Beaufour 2006-03-10 04:19:47 PST
Using repeats with ul/li makes FF crash:
###!!! ASSERTION: element not implementing nsIContent!?: 'content', file nsXTFFrameUtils.cpp, line 51
WARNING: NS_ENSURE_TRUE(aContent) failed: file nsFrameManager.cpp, line 338

Program dist/bin/firefox-bin (pid = 12691) received signal 11.
Stack:
UNKNOWN [dist/bin/libxul.so +0x0001F6CC]
__kernel_sigreturn+0x00000000 [ +0x00000420]
UNKNOWN [/store/mozilla/source/firefox/mozilla/dist/bin/components/libgklayout.so +0x000F5D36]
Comment 1 Allan Beaufour 2006-03-10 04:20:15 PST
Created attachment 214661 [details]
Testcase
Comment 2 aaronr 2006-03-10 08:59:35 PST
Debugged it.  Looks like the problem is that for nsXFormsRepeatElement we don't have a GetInsertionPoint so we are falling into the default one (nsXFormsXMLVisualStub::GetInsertionPoint()) which returns null.
Comment 3 aaronr 2006-03-10 11:51:13 PST
Created attachment 214705 [details] [diff] [review]
patch that fixes problem, but also traps on other testcases

This patch fixes the problem with the first trap and gives the desired behavior.  But it seems like no matter what I set the insertion point to be, we crash closing the browser after loading a different repeat form (which I'll attach after this) and then closing the browser.  Maybe a ref count thing?
Comment 4 aaronr 2006-03-10 11:52:43 PST
Created attachment 214706 [details]
testcase crashes with new patch

Apply the patch and build it.  Load this testcase then close the browser.  Browser crashes while closing.
Comment 5 Allan Beaufour 2006-03-11 04:22:37 PST
I remeber fixing that InsertionPoint thing... maybe I should start doing things, instead of apparently just envisioning it :)

Except for crashing I guess the patch is fine. But I've been crashing on exit a lot lately using trunk, without having time to investigate it. Are you sure it is related?

BTW.: I'm not even sure that it is legal to have <ul> + <repeat> + <li> -- for the same reasons as <table> + <repeat> + <tr> is illegal. But we shouldn't crash on it...
Comment 6 aaronr 2006-03-13 10:02:50 PST
(In reply to comment #5)

> Except for crashing I guess the patch is fine. But I've been crashing on exit a
> lot lately using trunk, without having time to investigate it. Are you sure it
> is related?
> 

I assume it is related since doing the same thing with this testcase w/o the patch doesn't crash.  But it is crashing during the javascript garbage collection so hard to pinpoint exactly what the object used to be that is no long behaving well.
Comment 7 Allan Beaufour 2006-04-21 04:33:04 PDT
Bug 306247 should take care of this.
Comment 8 Allan Beaufour 2006-04-26 03:03:16 PDT
This is fixed by bug 306247

Note You need to log in before you can comment on or make changes to this bug.