/Users/ap/WebKit/WebCore/rendering/RenderText.h:70: failed assertion `false' #3 0x01be97fc in WebCore::RenderText::layout at RenderText.h:70 #4 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #5 0x01992480 in WebCore::RenderContainer::layout at RenderContainer.cpp:410 #6 0x019fa5d0 in WebCore::RenderTable::layout at RenderTable.cpp:272 #7 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #8 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #9 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #10 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #11 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #12 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #13 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #14 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #15 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #16 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #17 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #18 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #19 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #20 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #21 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #22 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #23 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #24 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #25 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #26 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #27 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #28 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #29 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #30 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #31 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #32 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #33 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #34 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #35 0x01be518c in WebCore::RenderObject::layoutIfNeeded at Font.h:461 #36 0x0197e494 in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1073 #37 0x0197f024 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:495 #38 0x0196ebb4 in WebCore::RenderBlock::layout at RenderBlock.cpp:417 #39 0x0198d948 in WebCore::RenderCanvas::layout at RenderCanvas.cpp:114 #40 0x018bed3c in WebCore::FrameView::layout at FrameView.cpp:410
Created attachment 7842 [details] test case
Reduction: <div style="display: table-column-group;">a</div>
Created attachment 9006 [details] test case Same reduction, as an attachment.
*** Bug 12040 has been marked as a duplicate of this bug. ***
This assertion failure is also triggered by Hamachi, via different input: <script type="text/javascript"> window.onload = function() { var c = document.createElement('colgroup'); c.appendChild(document.createElement('br')); c.appendChild(document.createElement('xmp')); document.documentElement.appendChild(c); } </script>
<rdar://problem/4971212>
We'd really like iExploder and Hamachi to pass.
Created attachment 13127 [details] patch to fix the crash The patch fixes the crash. Since it disallows child renderers for table-col-group that aren't table-col's, it changes the result of two other test cases, but IMO this is more correct than the old output.
Comment on attachment 13127 [details] patch to fix the crash + RenderObject *r = createRenderer(document()->renderArena(), style); The * should go on the other side. + if (renderer()) { is redundant since you already null-checked r. Is it necessary to add a virtual function to fix this bug? I can think of two alternatives that don't add a virtual function: one is to implement RenderTableCol::layout() to just reset needsLayout and return, but then of course you will still have unneeded renderers in the tree (not sure if that's really a problem). Another option is to change RenderObject::addChild() to return a bool and allow it to reject the child by returning false, then override it in RenderTableCol to do the check. Have you considered these alternatives?
I don't think just reimplementing layout() in RenderTableCol is a good solution, as it leads to additional RenderObjects just lying around that could possibly mess up other dynamic manipulations of the render tree. In addition the table layouting routing has to make sure it ignores them. IMO it's cleaner design to catch these cases as early as possible. I also thought about adding a boolean return value to addChild, and we could do the patch that way, but that would require changing quite a bit more code. addChild() is called from quite a few places within the render tree as well. Obviously we could ignore the return value there, but that might lead to other issues in the long term.
Created attachment 13139 [details] new version of the patch Fixed the style issues.
Comment on attachment 13139 [details] new version of the patch r=me Consider replacing this with a single return statement: + if (child->isText() || !style || style->display() != TABLE_COLUMN) + return false; + return true;
Committed in r19582.