Summary: | page with use of first-letter crashes reproducibly in RenderObject::renderArena() | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jens Ayton <DHKIJUMWNCDB> | ||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Critical | CC: | alice.barraclough, bdakin, dacarson, darin, hyatt, mitz, nicholas | ||||||||||
Priority: | P1 | Keywords: | InRadar | ||||||||||
Version: | 312.x | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.3 | ||||||||||||
URL: | http://hol.istic.net/ | ||||||||||||
Attachments: |
|
Description
Jens Ayton
2005-06-15 23:12:34 PDT
Clarification: the page in question is that specified in the "URL" field of the report, i.e. http:// hol.istic.net/ The page with the example on it is mine. I'm going to leave that precise page alone until I can reproduce the exact bug on a static page (Downloading to desktop apparently stops the thing from crashing) for the purposes of debugging. If move the broken page (Pretty likely, as Jens mentioned I'm now on a powerbook, and my site crashing it is annoying ;-) I'll note here where it's gone. I'm getting a crash when I load the page in both TOT Webkit and Safari 2.0 (v412). Setting to P1 since this is a crash Attaching test case of problem. Created attachment 2411 [details]
Reduced test case from hol.istic.net
Opening this test case in Safari 2.0 or TOT Webkit results in a crash
The test case uses a script that affects several H3 elements in the BODY. The crash occurs when the first-letter pseudo selector is applied to one of these heading : <h3 id="versionsTitle">versionsTitle</h3>. When the script attempts to manipulating this element , a crash occurs. function versionsInit(){ thing = document.getElementById("revisionsList"); content = thing.innerHTML; if (content == ""){ document.getElementById("revisions").style.display = "none"; } else { header = document.getElementById("versionsTitle"); header.onclick = toggleVersions; header.content = header.innerHTML thing.style.display = "none"; header.innerHTML = "+ "+header.content; } } If you remove this first-letter pseudo selector, the crash no longer occurs. unable to reproduce Okay, From here (Safari 2.0.2 (416.12)) Clicking on the reduced test case crashes it out still (It no longer applies to Hol.istic.net itself since I Saw The Light, bought a Powerbook and started to care a lot more about being able to visit my own website in Safari) After a little debugging, the problem is obvious, but the solution is not yet obvious. The RenderTextFragment created for the first letter points to the same text node as the RenderTextFragment for the remaining text. But the text node only points to the remaining text renderer. Thus the text node's detach method only destroys the remaining text RenderTextFragment, orphaning a RenderTextFragment that now points to a non-existent node. We have to change something so that both renderers get destroyed. Maybe this can be special code in RenderTextFragment that knows about the other RenderTextFragment so that destroying one will destroy both. *** Bug 4269 has been marked as a duplicate of this bug. *** Created attachment 6892 [details]
no test case yet, but would like a first pass of review anyway
(In reply to comment #13) > Created an attachment (id=6892) [edit] Wrong patch or wrong bug? Comment on attachment 6892 [details]
no test case yet, but would like a first pass of review anyway
Oops. Attached this patch to the wrong bug!
Created attachment 7012 [details]
Proposed fix, no test and change log yet
I didn't bother with anything more generic than the first-letter case as it seems to be unique. Do you think firstLetter() belongs in a subclass (RenderTextFragmentRemaining?)?
Please r- since there's no test and change log.
Comment on attachment 7012 [details]
Proposed fix, no test and change log yet
Looks like a good fix to me.
Created attachment 7024 [details]
Same patch, with layout test and change log
I committed this patch. http://build.webkit.org/results/post-commit-leaks-powerpc-mac-os-x/598/DumpRenderTree-leaks.txt This patched causes RenderTextFragments to leak. Investigating... |