New issue
Advanced search Search tips

Issue 844744 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

CSS grid elements with justify-content: space-around have extra whitespace, sometimes a lot

Reported by kessbeth...@gmail.com, May 18 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

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.

Did this work before? N/A 

Does this work in other browsers? No
 Chrome, Safari, iOS Safari: Too much vertical whitespace appears at the bottom of the grid item.

Chrome version: 66.0.3359.139  Channel: n/a
OS Version: OS X 10.12.6
Flash Version: 

See also: https://stackoverflow.com/questions/49951135/chrome-skinny-css-grids-mystery-gap

jsfiddle: https://jsfiddle.net/kessbethler/22bmjfnc/6/
 
chrome-bug-test-case.html
7.6 KB View Download

Comment 1 by woxxom@gmail.com, May 18 2018

Bisected to 9979bfe2ae78b5b635cb5fc5d46b4d1ce2ad2436
"[CSS Grid Layout] Content Distirbution support for grid"
Landed in 44.0.2377.0 as r325902, disabled until Chrome 57.

Comment 2 by e...@chromium.org, May 18 2018

Cc: r...@igalia.com
Components: -Blink>CSS Blink>Layout>Grid
Status: Available (was: Unconfirmed)
Cc: jfernan...@igalia.com
Owner: jfernan...@igalia.com
Status: Assigned (was: Available)
I can confirm the bug and I found out is pretty related with an issue [1] we recently discussed with the spec editors. 

[1] https://github.com/w3c/csswg-drafts/issues/2557
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f

commit 0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f
Author: Javier Fernandez <jfernandez@igalia.com>
Date: Tue Jun 12 14:15:12 2018

[css-grid] Content Alignment as part of the track sizing algorithm

The CSS WG resolved [1] that Content Alignment should account to the
track sizing algorithm.

The sizing algorithm has been modified so that two new steps (1.5
and 2.5) were added to compute the Content Alignment offsets after
resolving the columns' and rows' sizes respectively.

This change decouples the Content Alignment logic from the tracks
position, so that we can use it as part of the track sizing algorithm.

I also had to store the whole ContentAlignmentData structure in two
class attributes. We need both, position and distribution offsets, to
be used in different parts of the layout logic.

Finally, since the ContentAlignmentData is now used as persistent
memory, we had to remove the STACK_ALLOCATED flag; hence, I think
using DISALLOW_COPY_AND_ASSIGN and let the functions update the
structure's internal field is a more efficient approach to commit
this change.

[1] https://github.com/w3c/csswg-drafts/issues/2557

Added WPT
  * grid-content-distribution-must-account-for-track-sizing-001.html
  * grid-content-distribution-must-account-for-track-sizing-002.html

Bug:  844744 
Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
Reviewed-on: https://chromium-review.googlesource.com/1067918
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#566412}
[add] https://crrev.com/0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html
[add] https://crrev.com/0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html
[modify] https://crrev.com/0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc
[modify] https://crrev.com/0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f/third_party/blink/renderer/core/layout/layout_grid.cc
[modify] https://crrev.com/0429cc5427bad3ca73d5dbd8731fdfbd563e4b4f/third_party/blink/renderer/core/layout/layout_grid.h

This issue should be FIXED now.
Status: Fixed (was: Started)

Sign in to add a comment