Bug 191308

Summary: CSS grid elements with justify-content: space-around have extra whitespace, sometimes a lot
Product: WebKit Reporter: Javier Fernandez <jfernandez>
Component: Layout and RenderingAssignee: Javier Fernandez <jfernandez>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, dino, jfernandez, rego, simon.fraser, svillar, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=844744
Attachments:
Description Flags
Patch
none
Patch none

Description Javier Fernandez 2018-11-06 08:49:19 PST
Steps to reproduce the problem:
1. Create a grid element where the total width of the columns is less than the width of the element itself.
2. Add justify-content: space-around to the grid element.
3. Add a child element with a large amount of text, and assign a very narrow overall width on the grid element, as if it were a mobile layout. This makes the bug more pronounced and easier to see.

What is the expected behavior?
The child element should only be as tall as the text inside it, as seen in Firefox.

What went wrong?
A gap appears at the bottom the child element, after the text.
Comment 1 Javier Fernandez 2018-11-06 09:11:37 PST
Created attachment 353971 [details]
Patch
Comment 2 Dean Jackson 2018-11-06 11:17:25 PST
Comment on attachment 353971 [details]
Patch

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

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:565
> +        // FIXME (jfernandez) Content Alignment should account for this heuristic

Nit. Punctuation.

> Source/WebCore/rendering/RenderGrid.cpp:1605
> +    auto& offset =
> +        isRowAxis ? m_offsetBetweenColumns : m_offsetBetweenRows;

Nit: Should be one line.

> Source/WebCore/rendering/RenderGrid.cpp:1624
> +        positionOffset = LayoutUnit();

I don't think this line is necessary.

> Source/WebCore/rendering/RenderGrid.cpp:1645
> +            positionOffset = LayoutUnit();

Nor this one.

> Source/WebCore/rendering/RenderGrid.h:42
> +    bool isValid() { return positionOffset >= 0 && distributionOffset >= 0; }

const
Comment 3 Javier Fernandez 2018-11-06 12:46:06 PST
Created attachment 353989 [details]
Patch
Comment 4 WebKit Commit Bot 2018-11-06 13:45:50 PST
Comment on attachment 353989 [details]
Patch

Clearing flags on attachment: 353989

Committed r237884: <https://trac.webkit.org/changeset/237884>
Comment 5 WebKit Commit Bot 2018-11-06 13:45:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2018-11-06 13:46:24 PST
<rdar://problem/45854716>