[webkit-reviews] review denied: [Bug 191462] [css-grid] Percentage height on replaced item with scrollbar : [Attachment 413803] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 11 04:09:27 PST 2020

Manuel Rego Casasnovas <rego at igalia.com> has denied zsun at igalia.com's request
for review:
Bug 191462: [css-grid] Percentage height on replaced item with scrollbar

Attachment 413803: Patch


--- Comment #2 from Manuel Rego Casasnovas <rego at igalia.com> ---
Comment on attachment 413803
  --> https://bugs.webkit.org/attachment.cgi?id=413803

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

Thanks for the patch, it looks good, but you need to fix some stuff before

> Source/WebCore/ChangeLog:9
> +	  

It's good to refer to the original patch in the ChangeLog, but we also should
explain and describe the change in this file. How the changes are fixing the

Apart from that, as we're now passing the test
css/css-grid/grid-items/percentage-size-replaced-subitems-001.html, we should
remove the line for that test in TestExpectations. So the test is actually run
by the bots.

> Source/WebCore/ChangeLog:11
> +	   Reviewed by NOBODY (OOPS!).

This line should be moved above, just after the URL bug line with an empty line
in the middle.

> Source/WebCore/rendering/RenderBox.cpp:3202
> +		       stretchedHeight = block->overridingLogicalHeight() -
block->borderAndPaddingLogicalHeight() - block->scrollbarLogicalHeight();

I guess you can use block->overridingContentLogicalHeight() directly here.

More information about the webkit-reviews mailing list