[webkit-reviews] review denied: [Bug 132601] [CSS Regions] Block incorrectly sized when containing an unsplittable box : [Attachment 231573] Patch

[bugzilla-daemon at webkit.org]

Andrei Bucur <abucur at adobe.com> has denied Radu Stavila <stavila at adobe.com>'s
request for review:
Bug 132601: [CSS Regions] Block incorrectly sized when containing an
unsplittable box
https://bugs.webkit.org/show_bug.cgi?id=132601

Attachment 231573: Patch
https://bugs.webkit.org/attachment.cgi?id=231573&action=review

------- Additional Comments from Andrei Bucur <abucur at adobe.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231573&action=review


> Source/WebCore/rendering/RenderBlockLineLayout.cpp:723
> +    if (view().layoutState() && view().layoutState()->isPaginated()) {

It should probably be best if this was a helper method on RenderBlockFlow with
the line box as a parameter.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:726
> +	       if (firstRun && firstRun->renderer().isBox() &&
toRenderBox(firstRun->renderer()).isUnsplittableForPagination()) {

What about cases when the line is really big (e.g. there's a span on the line
with a large font-size)? Anything placed on a line is unsplittable because the
line is unsplittable. I don't think having a box on the line is necessary to
trigger this "pull up" condition.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:727
> +		   LayoutUnit lineLogicalTop =
logicalTopForChild(toRenderBox(firstRun->renderer()));

Here you should use the information computed by the
RootInlineBox::alignBoxesInBlockDirection function. That function places the
line inside the RenderBlockFlow so after it is called you can use
lineBox->logicalTop().