[Webkit-unassigned] [Bug 209650] [css-grid] WPT Tests css/css-grid/grid-items/grid-item-percentage-sizes-*.html fail

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 22 04:06:41 PDT 2021


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

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

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

Looking good. I've some improvements in mind though

> Source/WebCore/ChangeLog:9
> +        percentage height against. Overridden containing block size should aslo be chosen in favor

Nits:
overriden->overriding
aslo->also

> Source/WebCore/ChangeLog:15
> +        Three tests that failed are now passed.

Nit: passed->passing

> Source/WebCore/rendering/RenderBox.cpp:3111
> +    const RenderBlock* const realContainingBlock = cb;

Blink uses overriding containing block sizes to stablish fences between LayoutNG and legacy code but that is not the case of WebKit. AFAIK only flex and grid set the overriding content logical sizes and in both cases cb would be equal to realContainingBlock because we wouldn't iterate the loop bellow that code as skipContainingBlockForPercentHeightCalculation() would return false independently on whether the item is orthogonal or quirks mode. That's why I think that we don't need the realContaininBlock variable and can directly use cb.

> Source/WebCore/rendering/RenderBox.cpp:3124
>  

We could perhaps define at this point a variable like:

bool isOrthogonal = isHorizontal != cb->isHorizontalWritingMode();

and use it in the following checks (either isOrthogonal or !isOrthogonal depending on the case).

> LayoutTests/ChangeLog:8
> +        * TestExpectations:

Nit: just mention here that grid-item-percentage-sizes-00{1-3}.html are unskipped because they're passing now.

-- 
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/20210422/13416019/attachment-0001.htm>


More information about the webkit-unassigned mailing list