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

Issue 2665983004: [css-grid] Fix static position of positioned grid items (Closed)

Created:
3 years, 10 months ago by Manuel Rego
Modified:
3 years, 10 months ago
CC:
chromium-reviews, jfernandez, szager+layoutwatch_chromium.org, zoltan1, svillar, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, Manuel Rego, leviw+renderwatch, jchaffraix+rendering, blink-reviews, eae+blinkwatch
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[css-grid] Fix static position of positioned grid items Modify LayoutGrid::prepareChildForPositionedLayout() to avoid including padding, to match the behavior described on the spec [1]: "The static position of an absolutely-positioned child of a grid container is determined as if it were the sole grid item in a grid area whose edges coincide with the padding edges of the grid container." The test is updated as the expected results were wrong. Note that after this patch we match Firefox behavior on this test. BUG=273898 TEST=fast/css-grid-layout/absolute-positioning-grid-container-parent.html [1] https://drafts.csswg.org/css-grid/#static-position Review-Url: https://codereview.chromium.org/2665983004 Cr-Commit-Position: refs/heads/master@{#447491} Committed: https://chromium.googlesource.com/chromium/src/+/13f00a7b6a700f280ec590e654cdbf223c510a45

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -8 lines) Patch
M third_party/WebKit/LayoutTests/fast/css-grid-layout/absolute-positioning-grid-container-parent.html View 2 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutGrid.cpp View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (4 generated)
Manuel Rego
3 years, 10 months ago (2017-02-01 11:07:08 UTC) #2
svillar
lgtm
3 years, 10 months ago (2017-02-01 11:22:06 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2665983004/1
3 years, 10 months ago (2017-02-01 11:28:23 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/13f00a7b6a700f280ec590e654cdbf223c510a45
3 years, 10 months ago (2017-02-01 12:36:50 UTC) #8
cbiesinger
3 years, 10 months ago (2017-02-01 19:59:30 UTC) #9
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698