[webkit-reviews] review granted: [Bug 15365] [CSS2.1] Anonymous tables should be inline/block-level based off their parent : [Attachment 153051] Proposed patch 2. Rebaselined insert-before-table-part-in-continuation.html as it is a progression.

[bugzilla-daemon at webkit.org]

Abhishek Arya <inferno at chromium.org> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 15365: [CSS2.1] Anonymous tables should be inline/block-level based off
their parent
https://bugs.webkit.org/show_bug.cgi?id=15365

Attachment 153051: Proposed patch 2. Rebaselined
insert-before-table-part-in-continuation.html as it is a progression.
https://bugs.webkit.org/attachment.cgi?id=153051&action=review

------- Additional Comments from Abhishek Arya <inferno at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=153051&action=review


> Source/WebCore/ChangeLog:25
> +	   intended as we will wrap the new block child into an inline table
per the check above.

Please add a line or two on how it does not impact the case for normal table.
As i understand it, it is because RenderObject::addChild will create the
anonymous block table and then we will go inside the check.

> Source/WebCore/rendering/RenderInline.cpp:306
> +    if (!newChild->isInline() &&
!newChild->isFloatingOrOutOfFlowPositioned() && !newChild->isTablePart()) {

clever trick.

> Source/WebCore/rendering/RenderTable.cpp:1297
> +    RefPtr<RenderStyle> newStyle =
RenderStyle::createAnonymousStyleWithDisplay(parent->style(),
parent->isInline() ? INLINE_TABLE : TABLE);

You can split the display to its own line and link the spec to it in a comment.


>
LayoutTests/platform/chromium-win/fast/dynamic/insert-before-table-part-in-cont
inuation-expected.txt:-1
> -layer at (0,0) size 785x748

can you check the test again with Firefox and Opera. There are still some
differences and I am curious what is causing that.