[Webkit-unassigned] [Bug 234578] Grid may be empty in certain scenarios

[bugzilla-daemon at webkit.org]

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

--- Comment #13 from Brandon <brandonstewart at apple.com> ---
Comment on attachment 450557
  --> https://bugs.webkit.org/attachment.cgi?id=450557
Patch

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

>> Source/WebCore/ChangeLog:8
>> +        Include check if a child should be excluded from the layout.
> 
> Let's improve this message. I'd write something like "Use OrderIterator's shouldSkipChild() to filter out items that should not be included in the aspect ratio dependant nor in the baseline lists. Just using isOutOfFlowPositioned() is not enough as there are some other cases of direct child of a grid element that are not grid items.". Feel free to use your own words and/or improve the grammar.

Updated the description.

>> Source/WebCore/rendering/RenderGrid.cpp:189
>> +void RenderGrid::clearChildrenOverridingLogicalHeight()
> 
> Nit: since we're filtering out non-grid items (like OOF children) I'd call this clearGridItemsOverriding...

Updated.

>> Source/WebCore/rendering/RenderGrid.cpp:223
>> +        child->clearOverridingLogicalHeight();
> 
> This block is duplicated

Removed. Thanks for catching this.

>> Source/WebCore/rendering/RenderGrid.cpp:240
>> +        // context during the track sizing algorithm.
> 
> I know this comes from the original code, but let's use longer lines instead of wrapping text so many times. I think we can use 3 lines instead of the current 6.

Reduced it down to 4 lines. 120 character line lengths.

>> Source/WebCore/rendering/RenderGrid.cpp:270
>> +        // instead of here.
> 
> Perhaps append "to save an extra loop over all grid items".

Updated comment

-- 
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/20220203/0fc4894f/attachment.htm>