New issue
Advanced search Search tips

Issue 846187 link

Starred by 4 users
Status: Fixed
Owner:
r...@igalia.com
Closed: Aug 7
Cc:
jfernan...@igalia.com
r...@igalia.com
svil...@igalia.com
robertma@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: ----
Type: ----



Sign in to add a comment

[css-grid] Percentage row tracks and gutters of indefinite height containers should be resolved during layout

Project Member Reported by monorail...@ecosystem-infra.iam.gserviceaccount.com, May 24 2018 
WPT import https://crrev.com/c/1070881 introduced new failures in external/wpt/css/css-grid:

List of new failures:
external/wpt/css/css-grid/alignment/grid-gutters-009.html [ Failure ]
external/wpt/css/css-grid/alignment/grid-gutters-010.html [ Failure ]

This import contains upstream changes from 232137f0fdacdeed99a7df5dd229d23020b0bccc to 314de955a5102650136404f6439f22f8d838e0f4:
Implement policy: 'document-stream-insertion': https://github.com/w3c/web-platform-tests/commit/314de955a5102650136404f6439f22f8d838e0f4
Split the layout test for remove-track: https://github.com/w3c/web-platform-tests/commit/cdec7db504d74c9a6f371a05d92f2be35ac8cc60
Update the gamepad IDL file (#9785): https://github.com/w3c/web-platform-tests/commit/3443ff6e255454aa23182b3765a1f9a2841c6a90
Allow Text node in elementsFromPoint if descendant of SVG text content: https://github.com/w3c/web-platform-tests/commit/65a5c24ba9769da8b103f72349ea111b124e54db [affecting this directory]
Add a simple test for ResourceTiming |name| with Service Workers: https://github.com/w3c/web-platform-tests/commit/9633890103f1a7025405cab78b00bb318ed9ebc4
Rename foreign-object-paints-before-rect-expected.html to -ref.html: https://github.com/w3c/web-platform-tests/commit/4698cffbd33ce8e20b8ae6c6c3032569999d78b2
Update the vibration IDL file (#9838): https://github.com/w3c/web-platform-tests/commit/badf1722b0794e9982324465b8fd65308abcc7f6
`Sec-Metadata` prototype.: https://github.com/w3c/web-platform-tests/commit/02e79502650e09e815cf3e835c5bfaef9a591bd0
Update the selection-api IDL file (#9829): https://github.com/w3c/web-platform-tests/commit/79e45ec83e0416793fdb6c59cd95d0acab63e5f4
Update the battery IDL file (#9761): https://github.com/w3c/web-platform-tests/commit/4ac0ab86ed4610e135f20de0820e3d220135fb2d
Merge pull request #11124 from w3c/sync_40a0ace6add33a4b01d49514f2ff747b69b4c85a: https://github.com/w3c/web-platform-tests/commit/dd43ad0f4b05f5ccce197d6ccd030d67039ca491
update "navigation within beforeunload" test to match the spec: https://github.com/w3c/web-platform-tests/commit/40a0ace6add33a4b01d49514f2ff747b69b4c85a
css-grid] Update a few WPT / reftests to new percentage row-gap layout.: https://github.com/w3c/web-platform-tests/commit/7f56040b8f31a6960003e66298eb2f755b03de9f [affecting this directory]
Run autopep8 on *.py in cors, fetch and xhr (#11116): https://github.com/w3c/web-platform-tests/commit/15b8e55fa2a0fbf0851cb8e0846e0320e319c852
Remove PaymentCurrencyAmount's currencySystem member (#11099): https://github.com/w3c/web-platform-tests/commit/741b59a02016570df79d117a808bf2cfadd4bbf2
Navigation fragment decode and encodings (#8723): https://github.com/w3c/web-platform-tests/commit/5b878a1e5de29aa4e68c48e0122878f983f036ff
URL/Encoding: change query state parsing: https://github.com/w3c/web-platform-tests/commit/e399a2c694345240639c23cc5e9e4f077a47cf30
FormData: Strings from form controls should be converted to USVStrings: https://github.com/w3c/web-platform-tests/commit/6df97f3630e83ec0caf1854b559c488368436a68
Initial Merge of WritableStreams Tests w/Upstream W3C (#6499): https://github.com/w3c/web-platform-tests/commit/a695026c7758c2f2d78aad1d2f511be5c9195b2d
Worker: Add service worker interception tests for module loading on dedicated workers: https://github.com/w3c/web-platform-tests/commit/347a7974c93326910109cd0f3386cc427add4ec5

Comment 1 by r...@igalia.com, May 24 2018

Cc: svil...@igalia.com jfernan...@igalia.com
Status: Available (was: Untriaged)
Summary: [css-grid] Row gap percentages should be resolved after layout (was: [WPT] New failures introduced in external/wpt/css/css-grid by import https://crrev.com/c/1070881)
Yes this is related to the change on how row-gap percentages are resolved.
They now should be resolved against the grid container height during layout.

The resolution is the one in this comment:
https://github.com/w3c/csswg-drafts/issues/509#issuecomment-386152310

The spec has been updated accordingly and Firefox has implemented it,
that's why the test have been modified and we're not passing them yet.

Comment 2 by r...@igalia.com, May 24 2018

Summary: [css-grid] Row gap percentages of indefinite height containers should be resolved during layout (was: [css-grid] Row gap percentages should be resolved after layout)

Comment 3 by r...@igalia.com, Jul 18

Owner: r...@igalia.com
Status: Started (was: Available)
Summary: [css-grid] Percentage row tracks and gutters of indefinite height containers should be resolved during layout (was: [css-grid] Row gap percentages of indefinite height containers should be resolved during layout)
The change is not only for gutters but also for percentage row tracks.

The CSSWG issues are:
* Percentage row tracks: https://github.com/w3c/csswg-drafts/issues/1921
* Percentage row gutters: https://github.com/w3c/csswg-drafts/issues/509
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 30 

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

commit 2f82bf0937e1ce8e7a88157d58ca5d525aad24c7
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Mon Jul 30 11:40:28 2018

[css-grid] Add deprecation message for the change on percentage rows

We're planning to update the behavior of percentage row tracks
and gutters when the grid container has an indefinite size
to match the last changes on the spec.

In the intent to implement and ship thread [1] it was agreed
to first add a deprecation message to notify web developers
about the upcoming change.

BUG= 846187 

[1] https://groups.google.com/a/chromium.org/d/msg/blink-dev/CJgcT4hR7Rk/58WfZNbWBQAJ

Change-Id: I5ca013615522d431c81bd96b6d12f61dd1846070
Reviewed-on: https://chromium-review.googlesource.com/1154531
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579014}
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-multiple-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-indefinite-calculated-height-crash-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-intrinsic-track-breadth-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-in-auto-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-in-minmax-crash-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-track-breadths-regarding-container-size-expected.txt
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/blink/renderer/core/frame/deprecation.cc
[modify] https://crrev.com/2f82bf0937e1ce8e7a88157d58ca5d525aad24c7/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc

Comment 5 by r...@igalia.com, Jul 30

Labels: Merge-Request-69
As discussed in the thread https://groups.google.com/a/chromium.org/d/msg/blink-dev/CJgcT4hR7Rk/58WfZNbWBQAJ
we'd like the deprecation message to appear in M69 already.

Comment 6 by gov...@chromium.org, Jul 30 

Pls apply appropriate OSs label.

Comment 7 by r...@igalia.com, Jul 31

Labels: OS-Android OS-Chrome OS-Fuchsia OS-iOS OS-Linux OS-Mac OS-Windows
> Pls apply appropriate OSs label.

This affects all platforms.
Project Member

Comment 8 by sheriffbot@chromium.org, Jul 31

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 31

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d21fecda25c6a868cd37caf0b81656d391c1669

commit 6d21fecda25c6a868cd37caf0b81656d391c1669
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Tue Jul 31 13:59:17 2018

[css-grid] Add deprecation message for the change on percentage rows

We're planning to update the behavior of percentage row tracks
and gutters when the grid container has an indefinite size
to match the last changes on the spec.

In the intent to implement and ship thread [1] it was agreed
to first add a deprecation message to notify web developers
about the upcoming change.

BUG= 846187 

[1] https://groups.google.com/a/chromium.org/d/msg/blink-dev/CJgcT4hR7Rk/58WfZNbWBQAJ

TBR=rego@igalia.com

(cherry picked from commit 2f82bf0937e1ce8e7a88157d58ca5d525aad24c7)

Change-Id: I5ca013615522d431c81bd96b6d12f61dd1846070
Reviewed-on: https://chromium-review.googlesource.com/1154531
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579014}
Reviewed-on: https://chromium-review.googlesource.com/1156604
Reviewed-by: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/branch-heads/3497@{#267}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-multiple-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-indefinite-calculated-height-crash-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-intrinsic-track-breadth-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-in-auto-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-in-minmax-crash-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-track-breadths-regarding-container-size-expected.txt
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/blink/renderer/core/frame/deprecation.cc
[modify] https://crrev.com/6d21fecda25c6a868cd37caf0b81656d391c1669/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 7 

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

commit 47bfc59ccdda82e2b28817a653c792d7a606ee8e
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Tue Aug 07 10:03:09 2018

[css-grid] Change how percentage row tracks and gaps are resolved

The CSSWG decided to change how percentage row tracks and gutters
in a grid container with indefinite height are resolved.

The CSSWG issues are:
* https://github.com/w3c/csswg-drafts/issues/1921
* https://github.com/w3c/csswg-drafts/issues/509

So far they were resolved as "auto", like it happens with
percentage heights in regular blocks. But now they're going to behave
similar to what happens in the columns axis, they would be ignored
to compute the intrinsic height.
This causes that we need to repeat the track sizing algorithm
when we have a grid container with indefinite height
that has some percentage rows using the intrinsic height
calculated on the first pass. Then the percentages will be resolved
against the intrinsic height.

We are adding two new tests for this new behavior
on top of updating several tests that were using percentages.
We also add a test for content alignment and the second pass
when the row size changes, the last case fails due to  crbug.com/871230 .

JFTR, intent to implement and ship thread:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/CJgcT4hR7Rk/58WfZNbWBQAJ

BUG= 846187 
TEST=css/css-grid/grid-definition/grid-percentage-rows-indefinite-height-001.html
TEST=css/css-grid/grid-definition/grid-percentage-rows-indefinite-height-002.html
TEST=css/css-grid/alignment/grid-content-alignment-second-pass-002.html

Change-Id: I2a1959af6c95e0c47d294580599fdbf9bc432348
Reviewed-on: https://chromium-review.googlesource.com/1142409
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#581185}
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-content-alignment-second-pass-002-expected.txt
[add] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-content-alignment-second-pass-002.html
[add] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-definition/grid-percentage-rows-indefinite-height-001.html
[add] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-definition/grid-percentage-rows-indefinite-height-002.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-multiple-expected.txt
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-multiple.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-container-percentage-rows.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-gutters-as-percentage.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-percentages-and-orthogonal-flows.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/named-grid-line-get-set.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-track-breadths-regarding-container-size.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-percentage-tracks.html
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/grid-columns-rows-get-set-multiple.js
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/grid-columns-rows-get-set.js
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.h
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/blink/renderer/core/layout/layout_grid.cc
[modify] https://crrev.com/47bfc59ccdda82e2b28817a653c792d7a606ee8e/third_party/blink/renderer/core/layout/layout_grid.h

Comment 11 by r...@igalia.com, Aug 7

Status: Fixed (was: Started)
This should be fixed now.
►

Sign in to add a comment