[Webkit-unassigned] [Bug 220733] [css-grid] max-height percentages are wrongly resolved for replaced grid items

[bugzilla-daemon at webkit.org]

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

--- Comment #6 from zsun at igalia.com ---
(In reply to Manuel Rego Casasnovas from comment #3)
> Comment on attachment 417951 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=417951&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        https://github.com/web-platform-tests/wpt/pull/26136
> 
> So there are different tests that we're interested here, maybe we should
> rewrite this somehow. I'm not sure if we need so many links.
Sorry! The second link doesn't belong here. Updated.

> 
> > Source/WebCore/ChangeLog:13
> > +        Reviewed by NOBODY (OOPS!).
> 
> Nit: This should be the line after the bug link and not here.
>
Updated.

> > Source/WebCore/ChangeLog:14
> > +
> 
> It's good to explain the changes in the ChangeLog, even when we have link to
> the Chromium commit, it's nice to explain here again what the patch is doing
> and why it's fixing the issue.
> 
Updated.

> > Source/WebCore/ChangeLog:15
> > +        Test: imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-img-item-percent-max-height-001.html
> 
> Is this patch only fixing this test, or also the other ones linked before?
> 
It only intends to fix this test.

> > Source/WebCore/rendering/RenderBox.cpp:3187
> > +        if (!document().inQuirksMode())
> 
> This is specifically checking something about quirks mode, is this a
> behavior change or is already covered by some existent quirks mode test.
> 
I'm not so sure. I just think that it handles differently in quirk mode and I haven't see any extra tests added in Chromium case.

> > LayoutTests/imported/w3c/ChangeLog:7
> > +
> 
> You can mention here, that you're importing this tests from WPT.
> 
Mentioned.

> Nit: They should be added to the corresponding w3c-import.log file.
> 
Only one test and it's been added.

> > LayoutTests/imported/w3c/ChangeLog:9
> > +        * web-platform-tests/css/css-grid/grid-items/grid-img-item-percent-max-height-001.html: Added.
> 
> Why only this test? There are a bunch more on the links in the ChangeLog.
> 
> Is this fix only for grid, or it also affects flexbox? If so we might need a
> flexbox test too.

It's a fix for grid only. I added some lines in ChangeLog to explain it. 

Thank you!

-- 
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/20210121/e755cca2/attachment.htm>