New issue
Advanced search Search tips

Issue 611410 link

Starred by 1 user
Status: Fixed
Owner:
r...@igalia.com
Closed: May 2016
Cc:
jfernan...@igalia.com
r...@igalia.com
svil...@igalia.com
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 611407
Blocking:
issue 79180
issue 611412



Sign in to add a comment

[css-grid] Flexible size as minimum in minmax() is invalid

Project Member Reported by r...@igalia.com, May 12 2016 
Another change [1] from the spec  that we should implement.
"fr" should be invalid if it's used in the min slot of minmax().

The new syntax on the spec is:
  <track-size> =
    <track-breadth> |
    minmax( <inflexible-breadth> , <track-breadth> )

  <inflexible-breadth> =
    <length> |
    <percentage> |
    min-content |
    max-content |
    auto

This means that a declaration like "grid-column: minmax(1fr, 100px);"
should be considered invalid.

Probably it's better to wait until  bug #611407  is solved before fixing this.
Otherwise we'd be rejecting "fr" explicitly on the min slot (like minmax(1fr, 100px)),
but accepting it implicitly as "1fr" would behave like minmax(1fr, 1fr)
instead of minmax(auto, 1fr).

[1] https://github.com/w3c/csswg-drafts/commit/4040ba7fa068905f7fe1c1bf35d665a2062acafa
[2] https://drafts.csswg.org/css-grid/#typedef-track-size

Comment 1 by r...@igalia.com, May 12 2016

Owner: r...@igalia.com
Status: Assigned (was: Available)
I've an initial patch to fix this,
but I'd wait for  bug #611407  before uploading it properly.

The draft patch is: http://sprunge.us/XdBf?diff

Comment 2 by r...@igalia.com, May 12 2016

Blocking: 611412
Project Member

Comment 3 by bugdroid1@chromium.org, May 16 2016 

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

commit 5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2
Author: rego <rego@igalia.com>
Date: Mon May 16 16:48:26 2016

[css-grid] Fix behavior of flexible track breadths

This patch is fixing 2 issues that are interrelated:

* Flex sizes are invalid on the min slot of minmax().

  The syntax has been recently updated on the spec:
    <track-size> =
      <track-breadth> |
      minmax( <inflexible-breadth> , <track-breadth> )

  To fix this, the patch modifies consumeGridBreadth()
  in CSSPropertyParser, adding a new type of restriction
  "InflexibleSizeOnly".

* Flex sizes outside minmax() behave as auto minimum.

  Flex sizes outside minmax() were previously behaving like
  minimum and maximum (e.g. 1fr => minmax(1fr, 1fr)).
  However the spec changed and now this would be invalid,
  so they should behave like auto minimum (e.g. minmax(auto, 1fr)).

  To achieve this the patch adds a new condition in
  LayoutGrid::gridTrackSize();

The patch includes new test cases checking specifically these 2 issues.
In addition several tests results have been updated to reflect
the new behavior. Also, some cases that are now invalid and
were not testing anything new have been removed.

BUG= 611407 , 611410 
TEST=fast/css-grid-layout/flex-content-resolution-columns.html
TEST=fast/css-grid-layout/flex-content-resolution-rows.html
TEST=fast/css-grid-layout/grid-columns-rows-get-set.html

Review-Url: https://codereview.chromium.org/1979603002
Cr-Commit-Position: refs/heads/master@{#393846}

[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-and-content-sized-resolution-columns-expected.txt
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-and-content-sized-resolution-columns.html
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-content-resolution-columns-expected.txt
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-content-resolution-columns.html
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-content-resolution-rows-expected.txt
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/flex-content-resolution-rows.html
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-gutters-and-flex-content-expected.txt
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-gutters-and-flex-content.html
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-preferred-logical-widths.html
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/non-grid-columns-rows-get-set-expected.txt
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/grid-columns-rows-get-set.js
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/LayoutTests/fast/css-grid-layout/resources/non-grid-columns-rows-get-set.js
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
[modify] https://crrev.com/5ab36ed9abb8f19c79d7c9bd09ca02460aad50c2/third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Comment 4 by r...@igalia.com, May 16 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment