Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(55)

Issue 1375173002: [css-grid] Unknown named grid lines on absolutely positioned items (Closed)

Created:
5 years, 2 months ago by Manuel Rego
Modified:
5 years, 2 months ago
CC:
blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-style_chromium.org, eae+blinkwatch, jchaffraix+rendering, jfernandez, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, Manuel Rego, svillar, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[css-grid] Unknown named grid lines on absolutely positioned items From a recent edition on the spec, unknown named grid lines on absolutely positioned grid items should be treated as "auto" (which corresponds to the padding edge of the grid container). More info at: https://drafts.csswg.org/css-grid/#abspos-items BUG=273898 TEST=fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html Committed: https://crrev.com/11673aa4216fbb6ba9185f544797902d30098e38 Cr-Commit-Position: refs/heads/master@{#351785}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Patch for landing #

Total comments: 8

Patch Set 3 : Fixed review comments #

Messages

Total messages: 19 (7 generated)
Manuel Rego
5 years, 2 months ago (2015-09-30 09:56:51 UTC) #2
cbiesinger
lgtm
5 years, 2 months ago (2015-09-30 14:14:01 UTC) #3
cbiesinger
https://codereview.chromium.org/1375173002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right): https://codereview.chromium.org/1375173002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp#newcode1436 third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1436: bool startIsAuto = startPosition.isAuto() || (startPosition.isNamedGridArea() && !GridResolvedPosition::isValidNamedLineOrArea(startPosition.namedGridLine(), *style(), ...
5 years, 2 months ago (2015-09-30 15:23:32 UTC) #4
Manuel Rego
Thanks for the review. https://codereview.chromium.org/1375173002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right): https://codereview.chromium.org/1375173002/diff/1/third_party/WebKit/Source/core/layout/LayoutGrid.cpp#newcode1436 third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1436: bool startIsAuto = startPosition.isAuto() || ...
5 years, 2 months ago (2015-09-30 19:49:59 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1375173002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1375173002/20001
5 years, 2 months ago (2015-09-30 19:51:07 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/builds/47663)
5 years, 2 months ago (2015-09-30 20:05:00 UTC) #10
Manuel Rego
Ups, I need an OWNER for the changes in core/style/. Please, could someone take a ...
5 years, 2 months ago (2015-09-30 20:56:30 UTC) #12
esprehn
lgtm w/ nits fixed https://codereview.chromium.org/1375173002/diff/20001/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html File third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html (right): https://codereview.chromium.org/1375173002/diff/20001/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html#newcode2 third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html:2: <html> leave out html https://codereview.chromium.org/1375173002/diff/20001/third_party/WebKit/Source/core/style/GridResolvedPosition.h ...
5 years, 2 months ago (2015-10-01 08:43:28 UTC) #13
Manuel Rego
Thanks for the review! https://codereview.chromium.org/1375173002/diff/20001/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html File third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html (right): https://codereview.chromium.org/1375173002/diff/20001/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html#newcode2 third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-items-unknown-named-grid-line.html:2: <html> On 2015/10/01 08:43:28, esprehn ...
5 years, 2 months ago (2015-10-01 11:29:25 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1375173002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1375173002/40001
5 years, 2 months ago (2015-10-01 11:30:20 UTC) #17
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 2 months ago (2015-10-01 12:59:58 UTC) #18
commit-bot: I haz the power
5 years, 2 months ago (2015-10-01 13:00:41 UTC) #19
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/11673aa4216fbb6ba9185f544797902d30098e38
Cr-Commit-Position: refs/heads/master@{#351785}

Powered by Google App Engine
This is Rietveld 408576698