Closed
Bug 473042
Opened 15 years ago
Closed 15 years ago
Crash [@ nsStyleContext::Destroy] with display:none root
Categories
(Core :: Layout, defect, P2)
Core
Layout
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: jruderman, Assigned: dbaron)
References
Details
(4 keywords, Whiteboard: [sg:critical] regression from 283686)
Crash Data
Attachments
(4 files, 2 obsolete files)
122 bytes,
application/xhtml+xml
|
Details | |
5.95 KB,
text/plain; charset=UTF-8
|
Details | |
6.71 KB,
text/plain; charset=UTF-8
|
Details | |
6.35 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
Crashes calling 0xdddddddd. Regression from bug 283686?
Flags: blocking1.9.1?
Reporter | ||
Updated•15 years ago
|
Whiteboard: [sg:critical]
Comment 1•15 years ago
|
||
I can reproduce this crash on Linux: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090112 Minefield/3.2a1pre http://crash-stats.mozilla.com/report/index/2afa9993-1e21-4a53-8508-29a9d2090112?p=1 http://crash-stats.mozilla.com/report/index/7e3d5691-1e82-4679-abf9-d08e02090112?p=1 OS --> All
OS: Mac OS X → All
Assignee | ||
Comment 2•15 years ago
|
||
Assignee | ||
Updated•15 years ago
|
Priority: -- → P2
Hardware: x86 → All
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → dbaron
Assignee | ||
Comment 3•15 years ago
|
||
Attachment #356626 -
Flags: superreview?(bzbarsky)
Attachment #356626 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•15 years ago
|
||
Oops, that assertion should be asserting about undisplayedParent, not localContent. I'll fix that locally.
Comment 5•15 years ago
|
||
Comment on attachment 356626 [details] [diff] [review] patch With the assert fixed, looks good.
Attachment #356626 -
Flags: superreview?(bzbarsky)
Attachment #356626 -
Flags: superreview+
Attachment #356626 -
Flags: review?(bzbarsky)
Attachment #356626 -
Flags: review+
Comment 6•15 years ago
|
||
Note that the bug that caused this behavior, bug 283686, is currnetly marked as "[needs 1.9.1 landing]". If/when that lands, we should make sure to land this as well.
Assignee | ||
Comment 7•15 years ago
|
||
Fixed: http://hg.mozilla.org/mozilla-central/rev/046f5da25280
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical] → [sg:critical][needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Comment 8•15 years ago
|
||
Backed out due to crash in layout/forms/test/ mochitests. http://hg.mozilla.org/mozilla-central/rev/dc598a5eafe1 http://hg.mozilla.org/mozilla-central/rev/1d21f44cf01c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [sg:critical][needs 1.9.1 landing] → [sg:critical][needs new patch]
Assignee | ||
Comment 9•15 years ago
|
||
Specifically, it's layout/forms/test/test_bug402198.html , which is actually toggling display:none on the root.
Assignee | ||
Comment 10•15 years ago
|
||
Assignee | ||
Comment 11•15 years ago
|
||
This has two additional changes from the previous patch. The first is the fix for the crash: the problem was that the primary frame for the root element was ending up being the canvas frame because the code to reset the primary frames for the scrollbars could end up, when no scrollbars were present, setting the primary frame for the root to be the canvas. This meant that, when the root was undisplayed and we reframed it, we'd delete the frame constructor's mDocElementContainingBlock without clearing the mDocElementContainingBlock pointer. The second change is the added call to ClearUndisplayedContentIn, which I think is also important. (I wrote that part first, actually.)
Attachment #356626 -
Attachment is obsolete: true
Assignee | ||
Comment 12•15 years ago
|
||
Here's a version that also has an assertion in SetPrimaryFrameFor.
Attachment #357018 -
Attachment is obsolete: true
Assignee | ||
Comment 13•15 years ago
|
||
And I checked that the assertion does fire without the ConstructDocElementFrame part of the patch.
Assignee | ||
Comment 14•15 years ago
|
||
Comment on attachment 357027 [details] [diff] [review] patch Ran full reftests and mochitests without crashing and without hitting my new assertion. (I think the failures I saw in mochitest are unrelated.)
Attachment #357027 -
Flags: superreview?(bzbarsky)
Attachment #357027 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical][needs new patch] → [sg:critical][needs review]
Comment 15•15 years ago
|
||
Comment on attachment 357027 [details] [diff] [review] patch r+sr=bzbarsky
Attachment #357027 -
Flags: superreview?(bzbarsky)
Attachment #357027 -
Flags: superreview+
Attachment #357027 -
Flags: review?(bzbarsky)
Attachment #357027 -
Flags: review+
Assignee | ||
Comment 16•15 years ago
|
||
Fixed: http://hg.mozilla.org/mozilla-central/rev/867b1cf06252
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical][needs review] → [sg:critical][needs 1.9.1 landing]
Updated•15 years ago
|
Flags: in-testsuite?
Flags: blocking1.9.1? → blocking1.9.1+
Assignee | ||
Comment 17•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d3e2e7c4578f
Keywords: fixed1.9.1
Whiteboard: [sg:critical][needs 1.9.1 landing] → [sg:critical]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
Comment 18•15 years ago
|
||
Verified fixed on trunk and 1.9.1 with builds on OS X and Windows: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090204 Minefield/3.2a1pre ID:20090204032912 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204034421
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1
Comment 19•15 years ago
|
||
I don't crash on 1.9.0.11pre, I assume we aren't going to take correctness bug 283686 on the 1.9.0 branch and therefore don't need this fix either.
Flags: wanted1.9.0.x-
Whiteboard: [sg:critical] → [sg:critical] regression from 283686
Comment 20•15 years ago
|
||
Incidentally, is this the right testcase? I get an XML error in both 1.9.0 and 1.9.1 -- my previous comment was based on adding the missing </mrow> to make the parser happy, but maybe that wasn't the point.
Updated•13 years ago
|
Crash Signature: [@ nsStyleContext::Destroy]
Updated•11 years ago
|
Group: core-security
Comment 21•10 years ago
|
||
Landed the crashtest: https://hg.mozilla.org/integration/mozilla-inbound/rev/3fa55f7a3d14
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•