[webkit-reviews] review granted: [Bug 111653] [CSS Grid Layout] Handle 2 positions with one 'auto' properly : [Attachment 191893] Proposed change 1: Added a new function to resolve both opposite positions. Naming could probably be improved (welcome any comment).

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 111653: [CSS Grid Layout] Handle 2 positions with one 'auto' properly
https://bugs.webkit.org/show_bug.cgi?id=111653

Attachment 191893: Proposed change 1: Added a new function to resolve both
opposite positions. Naming could probably be improved (welcome any comment).
https://bugs.webkit.org/attachment.cgi?id=191893&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191893&action=review


> Source/WebCore/rendering/RenderGrid.cpp:340
> +	   // No |positions| means that the item's position will need to be
resolved by the auto placement algoritm

Nit: algoritm -> algorithm

> Source/WebCore/rendering/RenderGrid.h:121
> +    // This is the function that should be used for any position resolution
as we *have* to
> +    // resolve both directions at the same time (e.g. auto / 1, span 2 / 4,
...).
> +    PassOwnPtr<GridSpan> resolveGridPositionsFromStyle(const RenderBox*,
TrackSizingDirection) const;

Nit: The wording of this comment is awkward.  Maybe the function name should
somehow suggest that this is for resolving both directions at the same time.