Closed
Bug 487539
Opened 16 years ago
Closed 16 years ago
[FIX]"ASSERTION: Shouldn't happen here: '!IsTablePseudo(inFlowFrame)'" with image map
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(4 keywords)
Attachments
(4 files)
967 bytes,
text/html
|
Details | |
1.30 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
980 bytes,
text/html
|
Details | |
3.30 KB,
patch
|
dbaron
:
approval1.9.1+
dveditz
:
approval1.9.0.11+
|
Details | Diff | Splinter Review |
###!!! ASSERTION: Shouldn't happen here: '!IsTablePseudo(inFlowFrame)', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 8826
Assignee | ||
Comment 1•16 years ago
|
||
Oh, this is more fun due to bug 135040, and the assertion is quite correct we're ending up with the wrong primary frame for the content node! The content model here before the removeChild call in boom2() is:
<map>
<area/>
<span>
<img/>
<td/>
</span>
</map>
The frames look like this:
Block(map)
Inline(span)
Image(img)
TableOuter(span)
Table(span)
RowGroup(span)
Row(span)
Cell(td)
Now we make a GetPrimaryFrameFor() call on the <span>. It's an inline, so not in the hashtable, so we look for the primary frame. Its previous sibling in the DOM is the <area>, so we pass its primary frame (the Image, due to bug 135040) as the hint.mPrimaryFrameForPrevSibling to FindPrimaryFrameFor. This calls FindFrameWithContent, and passes it that hint. So we get the next sibling of the hint frame, and lo and behold: its content node is the one we want (it's the anonymous outer table in that frame tree above). So we return it.
If I use GECKO_FRAMECTOR_DEBUG_FLAGS=fast-find-frame in my environment, we of course detect this issue immediately in FindPrimaryFrameFor:
##!! ASSERTION: hint shortcut found wrong frame: 'verifyTestFrame == *aFrame'
Assignee | ||
Comment 2•16 years ago
|
||
Oh, and this bug predates all the table changes; they just made the bug assert.
Assignee | ||
Comment 3•16 years ago
|
||
I wanted to add some asserts to FindFrameWithContent, but can't think of a reasonable thing to assert...
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #371889 -
Flags: superreview?(roc)
Attachment #371889 -
Flags: review?(roc)
Assignee | ||
Updated•16 years ago
|
Summary: "ASSERTION: Shouldn't happen here: '!IsTablePseudo(inFlowFrame)'" with image map → [FIX]"ASSERTION: Shouldn't happen here: '!IsTablePseudo(inFlowFrame)'" with image map
Assignee | ||
Comment 4•16 years ago
|
||
Jesse, does that test need to land as a crashtest, or is it already one of our crashtests?
Assignee | ||
Comment 5•16 years ago
|
||
I guess it could land as a reftest too.
Assignee | ||
Comment 6•16 years ago
|
||
On first load this works correctly, but on reload it doesn't make the image go away in Fx3...
I'd love to know how to make this into a reliable reftest!
Assignee | ||
Comment 7•16 years ago
|
||
Maybe with preloading the image before doing all that DOM creation? I suspect the key here is that it fails when the image doesn't have to reframe on load.
Reporter | ||
Comment 8•16 years ago
|
||
My testcase is not an existing automated test.
Attachment #371889 -
Flags: superreview?(roc)
Attachment #371889 -
Flags: superreview+
Attachment #371889 -
Flags: review?(roc)
Attachment #371889 -
Flags: review+
Assignee | ||
Comment 9•16 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/683b00c3d1d1, which includes a reftest that fails reliably on both Fx3 and trunk without this patch.
I think we should take this on both branches; without this we end up with frames mapping content that's no longer in the document, which can get a little weird, and the patch is quite safe, in my opinion.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee | ||
Comment 10•16 years ago
|
||
Attachment #371998 -
Flags: approval1.9.1?
Attachment #371998 -
Flags: approval1.9.0.10?
Attachment #371998 -
Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 371998 [details] [diff] [review]
As checked in; this applies to both branches after merging reftest.list
a1.9.1=dbaron
Assignee | ||
Comment 12•16 years ago
|
||
Keywords: fixed1.9.1
Comment 13•16 years ago
|
||
Comment on attachment 371998 [details] [diff] [review]
As checked in; this applies to both branches after merging reftest.list
Approved for 1.9.0.10, a=dveditz for release-drivers
Attachment #371998 -
Flags: approval1.9.0.10? → approval1.9.0.10+
Comment 15•16 years ago
|
||
Verified with attached testcase for 1.9.0.11 using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11pre) Gecko/2009051305 GranParadiso/3.0.11pre (.NET CLR 3.5.30729).
Keywords: fixed1.9.0.11 → verified1.9.0.11
You need to log in
before you can comment on or make changes to this bug.
Description
•