[Webkit-unassigned] [Bug 189513] Wrong static position for out-of-flow positioned element with different writing-mode than its containing block

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=189513

--- Comment #19 from Sergio Villar Senin <svillar at igalia.com> ---
Comment on attachment 428389
  --> https://bugs.webkit.org/attachment.cgi?id=428389
Patch

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

Looking better, I have a few other comments.

> Source/WebCore/rendering/RenderBox.cpp:3564
> +static inline bool childIsVLRParentInHTB(const RenderBox* child)

I think the name should be isVLRChildInHTBParent()

Also we know that the child parameter cannot be nullptr, so use a reference instead of a pointer.

Last but not least, I think we should pass a reference to the parent as well, that way the caller is responsible of providing non null values

> Source/WebCore/rendering/RenderBox.cpp:3568
> +}

supernit: it might be easier to understand what this condition is doing if we put the child checks first and the two parent ones later (that way we'd be matching the method's name).

> Source/WebCore/rendering/RenderBox.cpp:3604
> +                staticPosition += renderBox.logicalLeft();

We can use a ternary operator here
staticPosition += isVLRChildInHTBParent(child) ? renderBox.logicalTop() : renderBox.logicalLeft()

> Source/WebCore/rendering/RenderBox.cpp:4017
> +    TextDirection parentDirection = parent->style().direction();

We only use this to compare to TextDirection::LTR. Maybe we can just do

bool isParentDirectionLTR = parent->style().direction() == TextDirection::LTR;

> Source/WebCore/rendering/RenderBox.cpp:4033
> +                staticLogicalTop += renderBox.logicalTop();

Same here with regard to using a ternary operator

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210513/710f3485/attachment-0001.htm>