[Webkit-unassigned] [Bug 221337] Fix replaced element definiteness as a grid-item

[bugzilla-daemon at webkit.org]

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

--- Comment #6 from Javier Fernandez <jfernandez at igalia.com> ---
Comment on attachment 419408
  --> https://bugs.webkit.org/attachment.cgi?id=419408
Patch

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

>>> Source/WebCore/rendering/RenderBoxModelObject.cpp:285
>>> +        return thisBox->overridingContainingBlockContentLogicalHeight() == WTF::nullopt;
>> 
>> I don't understand why this change affects only to replaced grid items. Could you please elaborate on this ?
> 
> I think function hasAutoHeightOrContainingBlockWithAutoHeight() is only called by replaced handlers, right? I'm not sure why it's only for grid items though.

I missed that one of the clauses checks that this is indeed a grid item. However, I still have doubts about this change. 

I believe we indeed use in the GridTrackSizingAlgorithm logic a nullopt value for setting an indefinite size to the grid area (the grid item's override size). However, I've found some places where we still use -1 for that purpose (eg. RenderGrid::placeItemsOnGrid). I think nullopt is the correct choice, but I'd be more comfortable if we double check and correct the lines where we still use -1. I'd like to have layout tests to ensure the changes are verified.

Another idea to consider is that if this is a grid item, it shouldn't be valid that hasOverridingContainingBlockContentLogicalHeight(), since we initialize of the values, if I'm not wrong. We could add change this conditional clause for an assert.

-- 
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/20210222/e7abba26/attachment-0001.htm>