[webkit-reviews] review granted: [Bug 238854] [Multicol] Incorrect clipping when a layer is present between the column and the content layer : [Attachment 463629] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Rob Buis <rbuis at igalia.com>'s request
for review:
Bug 238854: [Multicol] Incorrect clipping when a layer is present between the
column and the content layer
https://bugs.webkit.org/show_bug.cgi?id=238854

Attachment 463629: Patch

https://bugs.webkit.org/attachment.cgi?id=463629&action=review




--- Comment #22 from Darin Adler <darin at apple.com> ---
Comment on attachment 463629
  --> https://bugs.webkit.org/attachment.cgi?id=463629
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:680
> +	       if (RenderFragmentedFlow* containingFragmentedFlow =
enclosingFragmentedFlow())

Could use “auto*” here, which would help readability. I might even call this
local just “flow” to make the two lines easier to read. More explicit might not
be important with something with such a narrow scope.

> Source/WebCore/rendering/RenderBox.cpp:4996
> +	   childVisualOverflowRect->move(delta);

Can’t we just put “+ delta” on the end of the previous line instead?

> Source/WebCore/rendering/RenderBox.cpp:5003
> +	   // Add in visual overflow from the child. Even if the child clips
its overflow, it may still

This comment seems to be in the wrong place now; might need to split and reword
it to be less confusing.

> LayoutTests/fast/multicol/positioned-split.html:1
> +<div style="-webkit-column-gap: 0;-webkit-column-count:2;
-webkit-column-fill:auto; column-count:2; column-fill:auto; border:2px solid
black; height:300px;">

Space after the semicolon?