Part 1 of fixing https://bugs.webkit.org/show_bug.cgi?id=82630
Created attachment 135511 [details] Patch
ah! forgot to remove createAnonymousTable from RenderBlock, in a function which will be eventually removed, but good as a cleanup in this patch itself. Also, just a fyi, that bidi-override-in-anonymous-block test is disabled on other platforms, so i didnt know the right rebaseline to do.
Created attachment 135574 [details] Patch
Comment on attachment 135574 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135574&action=review > Source/WebCore/rendering/RenderBlock.cpp:7425 > + if (display == BOX) { > + newStyle->setDisplay(BOX); > + newBox = new (parent->renderArena()) RenderDeprecatedFlexibleBox(parent->document() /* anonymous box */); It's too bad the rest of the code relies on that. This looks like another static function that could be split... > Source/WebCore/rendering/RenderBlock.h:238 > + static RenderBlock* createAnonymousColumnsWithParentRendererAndDisplay(const RenderObject*); > + static RenderBlock* createAnonymousColumnSpanWithParentRendererAndDisplay(const RenderObject*); Those 2 don't take a display so the name is wrong. This is not repeated but this is a wrong pattern in your naming. > Source/WebCore/rendering/RenderButton.cpp:53 > + bool isDeprecatedFlexibleBox = style()->display() == BOX || style()->display() == INLINE_BOX; > + m_inner = createAnonymousBlock(isDeprecatedFlexibleBox ? BOX : BLOCK); Isn't that a big FIXME that we actually don't honor our current display? I would actually pass the display as-is and push the INLINE_BOX display check into createAnonymousBlock along with the FIXME. > Source/WebCore/rendering/RenderTable.cpp:1280 > +RenderTable* RenderTable::createAnonymousWithParentRendererAndDisplay(const RenderObject* parent, EDisplay display) The only call site using that doesn't care about EDisplay. I think for now we should just drop the EDisplay argument unless needed (like for replacing the isFlexibleBox boolean). > Source/WebCore/rendering/RenderTableSection.cpp:1416 > +RenderTableSection* RenderTableSection::createAnonymousWithParentRendererAndDisplay(const RenderObject* parent, EDisplay display) I don't think you need to pass EDisplay here. Whenever you need to create an anonymous table section, it should use TABLE_ROW_GROUP as its display value.
Created attachment 135611 [details] Patch
Comment on attachment 135611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135611&action=review > Source/WebCore/ChangeLog:9 > + introducing a new static function createAnonymousWithParentRendererAndDisplay. s/createAnonymousWithParentRendererAndDisplay/createAnonymousWithParentRenderer/ > Source/WebCore/ChangeLog:10 > + The function builds a new anonymous wrapper inheriting style properties from a new anonymous wrapper of the same type as the class, inheriting style properties... > Source/WebCore/rendering/RenderBlock.cpp:7422 > + // FIXME: Why are we forcing a BOX display for old flex box here ? Is this a hack for RenderButton ? I think it should be stated more as: // FIXME: Do we need to convert all our inline displays to block-type in the anonymous logic? There is a function already doing that on CSSStyleSelector that we could reuse if that was going to happen (equivalentBlockDisplay). > LayoutTests/ChangeLog:11 > + * platform/chromium-mac/fast/css/bidi-override-in-anonymous-block-expected.txt: > + * platform/chromium-win/fast/css/bidi-override-in-anonymous-block-expected.txt: This needs to be skipped on all other platforms as you will be breaking them if not. For Chromium something like (to be double checked against our baselines): WK83116 LINUX WIN : fast/css/bidi-override-in-anonymous-block.html = TEXT For other platforms, it should just be added to the Skipped file.
Created attachment 135701 [details] Patch
Comment on attachment 135701 [details] Patch Clearing flags on attachment: 135701 Committed r113252: <http://trac.webkit.org/changeset/113252>
All reviewed patches have been landed. Closing bug.