New issue
Advanced search Search tips
Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocked on:
issue 799413

Blocking:
issue 798009



Sign in to add a comment

[css-grid] Rename gutter properties to remove "grid-" prefix

Project Member Reported by r...@igalia.com, Sep 5 2017

Issue description


The CSS WG has resolved to rename the gap properties:
* grid-column-gap to column-gap
* grid-row-gap to row-gap
* grid-gap to gap

Note that the default value is "normal" instead of "0".

Probably we need to keep the prefixed versions for a while (or forever) working as an alias of the new ones.
The new ones will eventually apply to Flexbox too.
 

Comment 1 by r...@igalia.com, Sep 5 2017

Link to the CSS WG GitHub issue: https://github.com/w3c/csswg-drafts/issues/1696

And the commit introducing the change on the spec: https://github.com/w3c/csswg-drafts/commit/8f243616df9b967ae546459e955ce0186188b54b

Comment 3 by r...@igalia.com, Dec 14 2017

There are a bunch of WPT tests failing because of this:
* external/wpt/css/css-grid/alignment/grid-gutters-001.html
* external/wpt/css/css-grid/alignment/grid-gutters-002.html
* ...
* external/wpt/css/css-grid/alignment/grid-gutters-010.html

Comment 4 by r...@igalia.com, Dec 28 2017

Blocking: 798009

Comment 5 by r...@igalia.com, Jan 4 2018

Cc: mstensho@chromium.org futhark@chromium.org

Some comments about this:

* Gutter properties are not animatable right now but "column-gap" is.
  Probably we should make them animatable together with the unprefix.

* I'm not 100% sure that we can have 2 aliases for the same property.
  We would need to have "-webkit-column-gap" and "grid-column-gap" working as alias of "column-gap".
  The spec actually says "grid-column-gap must be treated as a shorthand for the column-gap property"
  so maybe using an alias is not the right solution.

* "grid-column-gap" currently supports percentages, but "column-gap" doesn't support them.
  Only browser supporting percentages for "column-gap" is Edge.
  However these percentages (the ones in grid-column-gap and also the ones that Edge supports on column-gap)
  don't follow the last changes on the spec [1]: "Percentages resolve to zero when specified against
  a content-based size (such as the logical width of a float".
  And I'm not sure we can easily detect that situations in our current code [2],
  we'd to know during layout time if we're in content-based size element.

I'm adding some people on CC that can have some feedback about these topics.

[1] https://drafts.csswg.org/css-align-3/#column-row-gap
[2] https://github.com/w3c/csswg-drafts/issues/509#issuecomment-349294980
Alias is a Blink-internal thing, isn't it? Shorthand, on the other hand, is defined in the spec. So I suppose you need to make grid-column-gap a shorthand, not an alias.

Since the Box Alignment spec allows percentages for column-gap, I think it's time to go ahead and do it in Blink. It shouldn't be hard to implement percentages for multicol.

However, I think the spec needs to better clarify what it means with the percentage resolve to zero thing. I'll join the discussion.

Comment 7 by r...@igalia.com, Jan 4 2018

> Since the Box Alignment spec allows percentages for column-gap, I think it's time to go ahead and do it in Blink. It shouldn't be hard to implement percentages for multicol.

Yeah it shouldn't be specially hard.
I guess it'd be similar to what we do for grid, during the intrinsic size computation the column-gap is treated as "0px" (if you don't have a fixed width) and during layout it's resolved (like happens with percentage on regular blocks too).

> However, I think the spec needs to better clarify what it means with the percentage resolve to zero thing. I'll join the discussion.

Thanks for jumping into the discussion I also don't see how we can implement that.
For us the width is "indefinite" only during intrinsic size computation, but during layout we always have a width and we resolve percentages against that width.
The new text in the spec suggests that you shouldn't do that for a floated element (for example) and consider it has an indefinite width (which is not true).

Comment 8 by r...@igalia.com, Jan 5 2018

Blockedon: 799413

> Since the Box Alignment spec allows percentages for column-gap, I think it's time to go ahead and do it in Blink. It shouldn't be hard to implement percentages for multicol.

I've reported a specific issue to track this work, see  bug #799413 .

Comment 9 by r...@igalia.com, Jan 24 2018

Owner: r...@igalia.com
Status: Started (was: Available)
column-gap now accepts percentages, so I'll be working on unprefixing these properties.
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 5 2018

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

commit 406010c4b4e27073d52f2d7af881a302d1beae99
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Mon Feb 05 09:07:10 2018

[css-grid] Unprefix gutter properties

This patch applies the resoultion of the CSS WG to unprefix
the CSS Grid Layout gutter properties:
https://github.com/w3c/csswg-drafts/issues/1696

That is:
* grid-column-gap => column-gap
* grid-row-gap => row-gap
* grid-gap => gap

column-gap already existed before, as it's part of Multicol,
and it already has an alias -webkit-column-gap.
For that reason it's not possible to implement another alias
for grid-column-gap, so it was done with a shorthand.
To follow the same pattern the shorthand approach was used for
grid-row-gap and grid-gap too.

As column-gap was already animatable, this change takes advantage
to make animatable row-gap too.

Intent to Implement and Ship thread:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/UViBfJuuIq8/w7_2W7lLAgAJ

Converted grid-gutters-get-set.html in some WPT tests covering
a few extra cases.
Added WPT test to verify the animation of these properties too.

BUG= 761904 
TEST=external/wpt/css/css-align/gaps/

Change-Id: If49ec34116eff0b3b745fc89b01b15b14c71d4a9
Reviewed-on: https://chromium-review.googlesource.com/890446
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#534351}
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/animations/interpolation/row-gap-interpolation.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/column-gap-animation-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/column-gap-animation-002.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/column-gap-animation-003.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/column-gap-parsing-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/gap-animation-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/gap-animation-002.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/gap-animation-003.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/gap-animation-004.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/gap-parsing-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/grid-column-gap-parsing-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/grid-gap-parsing-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/grid-row-gap-parsing-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/row-gap-animation-001.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/row-gap-animation-002.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/row-gap-animation-003.html
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/external/wpt/css/css-align/gaps/row-gap-parsing-001.html
[delete] https://crrev.com/5fb546aaede6dca95bcc170fc681b86061849a18/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-gutters-get-set-expected.txt
[delete] https://crrev.com/5fb546aaede6dca95bcc170fc681b86061849a18/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-gutters-get-set.html
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-shorthand-get-set-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-shorthand-get-set.html
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/grid-definitions-parsing-utils.js
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/virtual/stable/webexposed/css-properties-as-js-properties-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/virtual/stable/webexposed/css-property-listing-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/webexposed/css-properties-as-js-properties-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/LayoutTests/webexposed/css-property-listing-expected.txt
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/animation/CSSInterpolationTypesMap.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/animation/LengthPropertyFunctions.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/BUILD.gn
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/CSSProperties.json5
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/CSSPropertiesRanking.json5
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/StylePropertySerializer.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/parser/CSSProtoConverter.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/CSSParsingUtils.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/CSSParsingUtils.h
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/ComputedStyleUtils.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/ComputedStyleUtils.h
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/longhands/ColumnGapCustom.cpp
[delete] https://crrev.com/5fb546aaede6dca95bcc170fc681b86061849a18/third_party/WebKit/Source/core/css/properties/longhands/GridColumnGapCustom.cpp
[delete] https://crrev.com/5fb546aaede6dca95bcc170fc681b86061849a18/third_party/WebKit/Source/core/css/properties/longhands/GridRowGapCustom.cpp
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/longhands/RowGapCustom.cpp
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/shorthands/GapCustom.cpp
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/shorthands/GridColumnGapCustom.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/shorthands/GridGapCustom.cpp
[add] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/css/properties/shorthands/GridRowGapCustom.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/frame/UseCounter.cpp
[modify] https://crrev.com/406010c4b4e27073d52f2d7af881a302d1beae99/third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Comment 12 by r...@igalia.com, Feb 5 2018

Status: Fixed (was: Started)

Sign in to add a comment