Closed Bug 473042 Opened 12 years ago Closed 12 years ago
Crash [@ ns
Style Context::Destroy] with display:none root
122 bytes, application/xhtml+xml
5.95 KB, text/plain; charset=UTF-8
6.71 KB, text/plain; charset=UTF-8
6.35 KB, patch
|Details | Diff | Splinter Review|
Crashes calling 0xdddddddd. Regression from bug 283686?
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
Oops, that assertion should be asserting about undisplayedParent, not localContent. I'll fix that locally.
Comment on attachment 356626 [details] [diff] [review] patch With the assert fixed, looks good.
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical] → [sg:critical][needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
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]
Specifically, it's layout/forms/test/test_bug402198.html , which is actually toggling display:none on the root.
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
Here's a version that also has an assertion in SetPrimaryFrameFor.
Attachment #357018 - Attachment is obsolete: true
And I checked that the assertion does fire without the ConstructDocElementFrame part of the patch.
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.)
Whiteboard: [sg:critical][needs new patch] → [sg:critical][needs review]
Comment on attachment 357027 [details] [diff] [review] patch r+sr=bzbarsky
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical][needs review] → [sg:critical][needs 1.9.1 landing]
Flags: blocking1.9.1? → blocking1.9.1+
Whiteboard: [sg:critical][needs 1.9.1 landing] → [sg:critical]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
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
I don't crash on 188.8.131.52pre, 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.
Whiteboard: [sg:critical] → [sg:critical] regression from 283686
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.
Crash Signature: [@ nsStyleContext::Destroy]
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.