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

Issue 200263003: [CSS Grid Layout] the "grid-template-areas" is not identified as computable property. (Closed)

Created:
6 years, 9 months ago by jfernandez
Modified:
6 years, 9 months ago
CC:
blink-reviews, ed+blinkwatch_opera.com, dglazkov+blink, apavlov+blink_chromium.org, darktears, rune+blink, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

[CSS Grid Layout] the "grid-template-areas" is not identified as computable property. The "grid-template-areas" property is a computable area, defined in the CSSComputedStyleDeclaration class with an specific computed value. However, it's not shown by the Web Inspector in the corresponding Computed tab. It seems that the root cause of this issue is that the property is not present in the CSSComputedStyleDeclaration::staticComputableProperties. BUG=79180 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170318

Patch Set 1 #

Patch Set 2 : Added layout test case. #

Total comments: 1

Patch Set 3 : Applied suggested changes. #

Total comments: 4

Patch Set 4 : Added DOCTYPE and html tags to the test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -0 lines) Patch
A LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html View 1 2 3 1 chunk +39 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout-expected.txt View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
M Source/core/css/CSSComputedStyleDeclaration.cpp View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
jfernandez
6 years, 9 months ago (2014-03-14 15:22:27 UTC) #1
eseidel
Please add a test, otherwise looks good.
6 years, 9 months ago (2014-03-14 16:25:12 UTC) #2
jfernandez
On 2014/03/14 16:25:12, eseidel wrote: > Please add a test, otherwise looks good. Test added. ...
6 years, 9 months ago (2014-03-17 11:01:29 UTC) #3
apavlov
https://codereview.chromium.org/200263003/diff/20001/LayoutTests/inspector/elements/styles/styles-computed-trace.html File LayoutTests/inspector/elements/styles/styles-computed-trace.html (right): https://codereview.chromium.org/200263003/diff/20001/LayoutTests/inspector/elements/styles/styles-computed-trace.html#newcode33 LayoutTests/inspector/elements/styles/styles-computed-trace.html:33: #id4 { This is not the right place to ...
6 years, 9 months ago (2014-03-21 16:50:39 UTC) #4
jfernandez
Patch rebased and suggested changes applied.
6 years, 9 months ago (2014-03-23 00:08:15 UTC) #5
apavlov
lgtm with comments https://codereview.chromium.org/200263003/diff/40001/LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html File LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html (right): https://codereview.chromium.org/200263003/diff/40001/LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html#newcode1 LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html:1: <script> I've seen a few cases ...
6 years, 9 months ago (2014-03-26 14:40:59 UTC) #6
jfernandez
https://codereview.chromium.org/200263003/diff/40001/LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html File LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html (right): https://codereview.chromium.org/200263003/diff/40001/LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html#newcode1 LayoutTests/fast/css/getComputedStyle/computed-style-grid-layout.html:1: <script> On 2014/03/26 14:40:59, apavlov wrote: > I've seen ...
6 years, 9 months ago (2014-03-28 00:28:38 UTC) #7
jfernandez
Applied part of the suggested changes. Still need some feedback before trying the CQ,
6 years, 9 months ago (2014-03-28 10:58:55 UTC) #8
apavlov
On 2014/03/28 10:58:55, jfernandez wrote: > Applied part of the suggested changes. > > Still ...
6 years, 9 months ago (2014-03-28 11:08:05 UTC) #9
jfernandez
The CQ bit was checked by jfernandez@igalia.com
6 years, 9 months ago (2014-03-28 11:14:15 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jfernandez@igalia.com/200263003/70001
6 years, 9 months ago (2014-03-28 11:14:23 UTC) #11
commit-bot: I haz the power
6 years, 9 months ago (2014-03-28 12:15:00 UTC) #12
Message was sent while issue was closed.
Change committed as 170318

Powered by Google App Engine
This is Rietveld 408576698