New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 799413 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 761904



Sign in to add a comment

[css-multicol] Support percentages in column-gap

Project Member Reported by r...@igalia.com, Jan 5 2018

Issue description


Edge is the only browser supporting percentages in "column-gap" at this point.
However all browsers support percentage in "grid-column-gap", which will be renamed to "column-gap" ( issue #761904 ).
So it seems it'd be a nice moment to add support in Multicolumn too.

The Multicolumn spec has not any special text regarding this (https://drafts.csswg.org/css-multicol/#column-gap),
however it's worth noticing the text in css-align spec (https://drafts.csswg.org/css-align-3/#column-row-gap):
  "Percentages resolve to zero when specified against a content-based size
   (such as the logical width of a float or the auto logical height of a block-level grid container)."

Note that there's some ongoing discussion with the CSS WG to clarify it:
https://github.com/w3c/csswg-drafts/issues/509#issuecomment-355242101

I believe the text in the spec needs to be changed for "column-gap",
as the width is always definite during layout, so we have to resolve the percentages.
At least I don't see how in Blink/WebKit we could detect the cases that try to cover the new text in the spec,
things like the width of a floated element is only indefinite during intrinsic size computation
(when the percentages are treated as 0)
but during layout we've a definite width to resolve percentages against it.
IMHO this should work like percentages widths work on regular blocks.
 

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

Blocking: 761904
Is it really blocking, though? Can't we just allow percentages in the CSS parser and store it as a Length (and not a float) in ComputedStyle? Multicol can just treat % as 0 until it gets implemented.

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

We could do that, but IMHO it'd be strange that the paring is ok and then nothing happens when you use a percentage.

We're not in a hurry to unprefix the other properties that's why I marked the dependency, but it's more a nice to have than a real block as you said.
Also we could help implementing this, but first we need the input from the CSS WG.

Other option would be to implement it just like we think it should be, resolving the percentages always (like for regular blocks).
That's what Edge is currently shipping for example, and what Chromium, Safari and Edge are currently shipping for grid-column-gap too.
And at some point in the future update the implementation if the CSS WG clarifies things and a different behavior is agreed.
Yeah, parsing percents but not acting upon it in multicol would be a bug (this bug), but a short-lived one. I promise. :) If somebody wants to do the plumbing in css/ and style/ , I should be able to fix it quickly for multicol (as long as I have a ComputedStyle::ColumnGap() that can return percents).

Comment 5 by e...@chromium.org, Jan 7 2018

Status: Available (was: Untriaged)

Comment 6 by r...@igalia.com, Jan 19 2018

Owner: r...@igalia.com
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 23 2018

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

commit 0eb06307382a59b1d66afa66703cdc95a2b7ca2e
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Tue Jan 23 21:29:40 2018

[css-multicol] Support percentages in column-gap

This patch adds percentage support to column-gap property.

Most of the changes are related to the parsing logic,
the column-gap property now accepts both length and percentages,
on top of the "normal" initial value.
A new utility class GapLength has been added, as it'll be useful
to implement row-gap in the future.

Apart from that the muticolumn layout code has been modified
to resolve the percentage gaps (treating them as zero while computing
preferred widths) and resolving them during layout.
This doesn't follow the current text on the spec, but there is an
ongoing discussion that might cause the text is changed:
https://github.com/w3c/csswg-drafts/issues/509#issuecomment-355242101
We could update the implementation once we have a definitive answer
from the CSS WG.

Added a new WPT test to check the behavior under different
sizing conditions.

BUG= 799413 
TEST=external/wpt/css/css-multicol/multicol-gap-percentage-001.html

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Icccd046e913353b6f525481046a41ad125aea5ff
Reviewed-on: https://chromium-review.googlesource.com/878199
Reviewed-by: meade_(do not use) <meade@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531356}
[add] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-gap-percentage-001.html
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/build/scripts/core/css/properties/make_css_property_subclasses.py
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/build/scripts/make_style_builder.py
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/animation/LengthPropertyFunctions.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/css/CSSProperties.json5
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/css/properties/longhands/ColumnGapCustom.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.h
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.h
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/layout/LayoutMultiColumnSet.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/layout/ng/ng_column_layout_algorithm.cc
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/layout/ng/ng_length_utils.cc
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/layout/ng/ng_length_utils.h
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/style/BUILD.gn
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/style/ComputedStyle.cpp
[modify] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/style/ComputedStyle.h
[add] https://crrev.com/0eb06307382a59b1d66afa66703cdc95a2b7ca2e/third_party/WebKit/Source/core/style/GapLength.h

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

I'm closing this, if finally we need to do some changes due to CSSWG discussions we could create a new bug to track that work.

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

Status: Fixed (was: Started)

Sign in to add a comment