New issue
Advanced search Search tips

Issue 915551 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

[css-grid] Line names are resolved incorrectly for grid-aligned abs.pos. child

Project Member Reported by mpalmg...@mozilla.com, Dec 16

Issue description

Chrome Version: 72.0.3595.2 (Official Build) dev (64-bit)
OS: Linux

What steps will reproduce the problem?
(1)load the attached testcase
(2)
(3)

What is the expected result?
A 50px green square and no red areas.  IOW, "foo" should be resolved
the same as for the in-flow grid item since there is an implicit line
named "foo".  The spec is very clear IMO that abs.pos. children
can address the full implicit grid, not just the explicit grid.

See Firefox and Edge (EdgeHTML version) for correct results.

 
Chrome-bug-abspos-resolving-to-implicit-line.html
801 bytes View Download
Summary: [css-grid] Line names are resolved incorrectly for grid-aligned abs.pos. child (was: [css-grid] Non-existent line names are resolved incorrectly for grid-aligned abs.pos. child)
Cc: jfernan...@igalia.com r...@igalia.com
Status: Available (was: Untriaged)

The relevant spec texts are:
* 8.3. Line-based Placement (https://drafts.csswg.org/css-grid/#grid-placement-int):
  "all implicit grid lines are assumed to have that name
   for the purpose of finding this position."

Only checking this part of the spec it seems pretty clear that Firefox behavior is the good one.

* 9.1. With a Grid Container as Containing Block (https://drafts.csswg.org/css-grid/#abspos-items):
  "If a grid-placement property refers to a non-existent line
   either by explicitly specifying such a line
   or by spanning outside of the existing implicit grid,
   it is instead treated as specifying auto (instead of creating new implicit grid lines)."

However this part makes me wonder we should use "auto" line or the implicit lines in this example.
"referst to a non-existent line", if there are implicit lines that's not possible
as all the implicit lines are assumed to have that name.
For example, if we use "bar" instaed of "foo" for the abspos in the example.
Should we add some clarifications to that sentence?
Filled a CSSWG to ask about this: https://github.com/w3c/csswg-drafts/issues/3445

A note has been added to the spec to clarify the 9.1. section.
So I agree it's a bug on our side.
Cc: obru...@igalia.com
Owner: obru...@igalia.com
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 10

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

commit 80068ae883fa0ba5ae2714c6743191bc49acece9
Author: Oriol Brufau <obrufau@igalia.com>
Date: Thu Jan 10 13:00:23 2019

[css-grid] Let abspos items reference implicit grid lines

While they can't create new implicit grid lines, abspos items
can reference existing ones as clarified in
https://github.com/w3c/csswg-drafts/commit/511bb63

This patch makes Blink match Firefox and Edge.

Spec: https://drafts.csswg.org/css-grid/#abspos-items

BUG= 915551 

TEST=external/wpt/css/css-grid/abspos/grid-positioned-items-padding-001.html
TEST=external/wpt/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html

Change-Id: Ib363d6cc5c4ec9cc584ebd4a86c33719cea9f54c
Reviewed-on: https://chromium-review.googlesource.com/c/1403656
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#621556}
[modify] https://crrev.com/80068ae883fa0ba5ae2714c6743191bc49acece9/third_party/blink/renderer/core/layout/layout_grid.cc
[modify] https://crrev.com/80068ae883fa0ba5ae2714c6743191bc49acece9/third_party/blink/renderer/core/layout/layout_grid.h
[modify] https://crrev.com/80068ae883fa0ba5ae2714c6743191bc49acece9/third_party/blink/renderer/core/style/grid_positions_resolver.cc
[modify] https://crrev.com/80068ae883fa0ba5ae2714c6743191bc49acece9/third_party/blink/renderer/core/style/grid_positions_resolver.h
[modify] https://crrev.com/80068ae883fa0ba5ae2714c6743191bc49acece9/third_party/blink/web_tests/external/wpt/css/css-grid/abspos/grid-positioned-items-padding-001.html
[modify] https://crrev.com/80068ae883fa0ba5ae2714c6743191bc49acece9/third_party/blink/web_tests/external/wpt/css/css-grid/abspos/grid-positioned-items-unknown-named-grid-line-001.html

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 10

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

commit 702e9fc024f93d2525d83114516b7cdba6bcc308
Author: Oriol Brufau <obrufau@igalia.com>
Date: Thu Jan 10 18:35:43 2019

[css-grid] Remove unused parameters after r621556

The grid_container_style parameter of InitialAndFinalPositionsFromStyle
and SpanSizeForAutoPlacedItem is no longer used after since r621556 and
can be removed.

BUG= 915551 

Change-Id: Iac8baaf86233d9fce6ae70dff8ff5e539e0d18ca
Reviewed-on: https://chromium-review.googlesource.com/c/1405308
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#621658}
[modify] https://crrev.com/702e9fc024f93d2525d83114516b7cdba6bcc308/third_party/blink/renderer/core/layout/layout_grid.cc
[modify] https://crrev.com/702e9fc024f93d2525d83114516b7cdba6bcc308/third_party/blink/renderer/core/style/grid_positions_resolver.cc
[modify] https://crrev.com/702e9fc024f93d2525d83114516b7cdba6bcc308/third_party/blink/renderer/core/style/grid_positions_resolver.h

Sign in to add a comment