Bug 58557 – Extract layoutRepacedElements from layoutInlineChildren

• Status: NEW
• Component: Layout and Rendering

[Ryosuke Niwa]

This is a cleanup.

[Ryosuke Niwa]

Created attachment 89612 [details]
cleanup

[Eric Seidel (no email)]

Comment on attachment 89612 [details]
cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:716
> +inline void RenderBlock::layoutRepacedElements(bool relayoutChildren, bool fullLayout, Vector<RenderBlock::FloatWithRect>& floats, bool& hasInlineChild)

I thought about doing just this.  HOweer, this does more than just layout replaced elments.  It's sorta a first-pass layout.  I wasn't really sure what to call it when I wrote a similar patch.

[Daniel Bates]

Comment on attachment 89612 [details]
cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review

> Source/WebCore/ChangeLog:8
> +        Extracted a block of code in layoutInlineChildren as layoutRepacedElements.

layoutRepacedElements => layoutReplacedElements

(you used layoutRepacedElements throughout this patch)

And, as Eric Seidel remarked, this function seems to more than just handle replace elements.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:740
> +        } else if (object->isText() || (object->isRenderInline() && !endOfInline)) {

Nit: The parentheses around the second disjunct are unnecessary since && (logical and) has higher precedence than || (logical or).

[Ryosuke Niwa]

Comment on attachment 89612 [details]
cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=89612&action=review

>> Source/WebCore/ChangeLog:8
>> +        Extracted a block of code in layoutInlineChildren as layoutRepacedElements.
> 
> layoutRepacedElements => layoutReplacedElements
> 
> (you used layoutRepacedElements throughout this patch)
> 
> And, as Eric Seidel remarked, this function seems to more than just handle replace elements.

Oops. I'll rename the function.  Any suggestion?

>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:740
>> +        } else if (object->isText() || (object->isRenderInline() && !endOfInline)) {
> 
> Nit: The parentheses around the second disjunct are unnecessary since && (logical and) has higher precedence than || (logical or).

There's at least one port where not having this parenthesis causes a build failure because new versions of gcc gives you a warning on this.

[Eric Seidel (no email)]

Comment on attachment 89612 [details]
cleanup

We need a better name.  We'll work on this mor in our hackathon on thurs.