New issue
Advanced search Search tips

Issue 788570 link

Starred by 3 users

Issue metadata

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

Blocking:
issue 545318



Sign in to add a comment

[css-typed-om] Implement CSSStyleValue normalization

Project Member Reported by shend@chromium.org, Nov 26 2017

Issue description

Some of this is implemented, but math values, position values and unparsed values are not.

Spec: https://drafts.css-houdini.org/css-typed-om-1/#stylevalue-normalization
 

Comment 1 by shend@chromium.org, Nov 27 2017

Labels: -Hotlist-Interop
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 30 2017

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

commit 86e5b818d7f465e9b368e50de28d7eb2abce22d7
Author: Darren Shen <shend@chromium.org>
Date: Thu Nov 30 01:07:55 2017

[css-typed-om] Implement CSSNumericValue.parse.

This patch implements CSSNumericValue.parse. We reuse the existing
calc() parser, modifying it to keep bracket information since the result
of CSSNumericValue.parse is sensitive to brackets. We also have to
merge certain nodes in the parsed calc tree (e.g. calc(1 + 2 + 3) gets
merged into one sum node).

Spec: https://drafts.css-houdini.org/css-typed-om-1/#dom-cssnumericvalue-parse

Bug:  776173 ,  788570 
Change-Id: Ia4bef7c3a2eb580d11a5e51d3921ed52e1f17bf3
Reviewed-on: https://chromium-review.googlesource.com/792670
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520361}
[add] https://crrev.com/86e5b818d7f465e9b368e50de28d7eb2abce22d7/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/numeric-objects/parse.html
[modify] https://crrev.com/86e5b818d7f465e9b368e50de28d7eb2abce22d7/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp
[modify] https://crrev.com/86e5b818d7f465e9b368e50de28d7eb2abce22d7/third_party/WebKit/Source/core/css/CSSCalculationValue.h
[modify] https://crrev.com/86e5b818d7f465e9b368e50de28d7eb2abce22d7/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.cpp
[modify] https://crrev.com/86e5b818d7f465e9b368e50de28d7eb2abce22d7/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.h
[modify] https://crrev.com/86e5b818d7f465e9b368e50de28d7eb2abce22d7/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2017

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

commit 19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Nov 30 03:47:49 2017

Revert "[css-typed-om] Implement CSSNumericValue.parse."

This reverts commit 86e5b818d7f465e9b368e50de28d7eb2abce22d7.

Reason for revert: Added test is failing on Mac, e.g.
https://storage.googleapis.com/chromium-layout-test-archives/Mac10_11_Tests/20861/layout-test-results/results.html

FAIL Parsing a calc with mixed compatible units returns correct CSSMathValue assert_equals: expected "px" but got "number"

Original change's description:
> [css-typed-om] Implement CSSNumericValue.parse.
> 
> This patch implements CSSNumericValue.parse. We reuse the existing
> calc() parser, modifying it to keep bracket information since the result
> of CSSNumericValue.parse is sensitive to brackets. We also have to
> merge certain nodes in the parsed calc tree (e.g. calc(1 + 2 + 3) gets
> merged into one sum node).
> 
> Spec: https://drafts.css-houdini.org/css-typed-om-1/#dom-cssnumericvalue-parse
> 
> Bug:  776173 ,  788570 
> Change-Id: Ia4bef7c3a2eb580d11a5e51d3921ed52e1f17bf3
> Reviewed-on: https://chromium-review.googlesource.com/792670
> Reviewed-by: nainar <nainar@chromium.org>
> Commit-Queue: Darren Shen <shend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520361}

TBR=nainar@chromium.org,shend@chromium.org

Change-Id: Ib2acc1f4fc07390c75ecdda9991f984a55009a81
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  776173 ,  788570 
Reviewed-on: https://chromium-review.googlesource.com/798991
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520421}
[delete] https://crrev.com/19ff5882db85e869c446f7c5218bbcda35bf45bd/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/numeric-objects/parse.html
[modify] https://crrev.com/19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp
[modify] https://crrev.com/19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d/third_party/WebKit/Source/core/css/CSSCalculationValue.h
[modify] https://crrev.com/19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.cpp
[modify] https://crrev.com/19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.h
[modify] https://crrev.com/19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 1 2017

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

commit 2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f
Author: Darren Shen <shend@chromium.org>
Date: Fri Dec 01 01:12:52 2017

Revert "Revert "[css-typed-om] Implement CSSNumericValue.parse.""

This reverts commit 19f5db9f5d50249fe5ce8b06cbf2dc1a7c9a645d.

Reason for revert: Trying to reland

Original change's description:
> Revert "[css-typed-om] Implement CSSNumericValue.parse."
>
> This reverts commit 86e5b818d7f465e9b368e50de28d7eb2abce22d7.
>
> Reason for revert: Added test is failing on Mac, e.g.
> https://storage.googleapis.com/chromium-layout-test-archives/Mac10_11_Tests/20861/layout-test-results/results.html
>
> FAIL Parsing a calc with mixed compatible units returns correct CSSMathValue assert_equals: expected "px" but got "number"
>
> Original change's description:
> > [css-typed-om] Implement CSSNumericValue.parse.
> >
> > This patch implements CSSNumericValue.parse. We reuse the existing
> > calc() parser, modifying it to keep bracket information since the result
> > of CSSNumericValue.parse is sensitive to brackets. We also have to
> > merge certain nodes in the parsed calc tree (e.g. calc(1 + 2 + 3) gets
> > merged into one sum node).
> >
> > Spec: https://drafts.css-houdini.org/css-typed-om-1/#dom-cssnumericvalue-parse
> >
> > Bug:  776173 ,  788570 
> > Change-Id: Ia4bef7c3a2eb580d11a5e51d3921ed52e1f17bf3
> > Reviewed-on: https://chromium-review.googlesource.com/792670
> > Reviewed-by: nainar <nainar@chromium.org>
> > Commit-Queue: Darren Shen <shend@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#520361}
>
> TBR=nainar@chromium.org,shend@chromium.org
>
> Change-Id: Ib2acc1f4fc07390c75ecdda9991f984a55009a81
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  776173 ,  788570 
> Reviewed-on: https://chromium-review.googlesource.com/798991
> Reviewed-by: Timothy Loh <timloh@chromium.org>
> Commit-Queue: Timothy Loh <timloh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520421}

Change-Id: I204827390b8de101f7868aeb4c384ed8fa649fd9
Bug:  776173 ,  788570 
Reviewed-on: https://chromium-review.googlesource.com/798992
Reviewed-by: nainar <nainar@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520782}
[add] https://crrev.com/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/numeric-objects/parse.html
[modify] https://crrev.com/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp
[modify] https://crrev.com/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f/third_party/WebKit/Source/core/css/CSSCalculationValue.h
[modify] https://crrev.com/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.cpp
[modify] https://crrev.com/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.h
[modify] https://crrev.com/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 6 2017

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

commit 0365acdcbdebed0896f9078f3dc62163ce4ae211
Author: Darren Shen <shend@chromium.org>
Date: Wed Dec 06 02:20:58 2017

[css-typed-om] Normalize CSS variables in property values.

When we specify CSS variables in a property value (e.g. color: var(--A))
we need to return a CSSUnparsedValue. This patch implements this
by first trying to parse the property value according to the property's
grammar. If it fails, we then check if it has var references and parse
it as a variable reference. The code for converting a variable reference
to a CSSUnparsedValue was already in the codebase so we just hook into
that.

Bug:  788570 
Change-Id: I65b32d29e80918c61e1731e7a0424b72e7d9b1d7
Reviewed-on: https://chromium-review.googlesource.com/804980
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521955}
[add] https://crrev.com/0365acdcbdebed0896f9078f3dc62163ce4ae211/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-tokens.html
[modify] https://crrev.com/0365acdcbdebed0896f9078f3dc62163ce4ae211/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 6 2017

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

commit 47b0b6ca47de80704b6ac0076bdf98a11b29b006
Author: Darren Shen <shend@chromium.org>
Date: Wed Dec 06 02:46:38 2017

[css-typed-om] Normalize custom property declarations.

When parsing custom properties or retrieving them from CSSOM, we need
to return a CSSUnparsedValue.

Spec: https://drafts.css-houdini.org/css-typed-om-1/#normalize-tokens

Bug:  788570 ,  779477 
Change-Id: Ia2fa289dc076ffd6dbacf03822907663209c6284
Reviewed-on: https://chromium-review.googlesource.com/804920
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521969}
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/LayoutTests/external/wpt/css/css-paint-api/style-background-image.https.html
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/LayoutTests/external/wpt/css/css-paint-api/style-before-pseudo.https.html
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-tokens.html
[delete] https://crrev.com/fe2b322b600dd7332610f601800722ae4fff00e6/third_party/WebKit/LayoutTests/typedcssom/stylevalue-objects/parse-parseAll-expected.txt
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/LayoutTests/typedcssom/stylevalue-objects/parse-parseAll.html
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/computed-expected.txt
[delete] https://crrev.com/fe2b322b600dd7332610f601800722ae4fff00e6/third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/get-expected.txt
[delete] https://crrev.com/fe2b322b600dd7332610f601800722ae4fff00e6/third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/getAll-expected.txt
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.cpp
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.h
[modify] https://crrev.com/47b0b6ca47de80704b6ac0076bdf98a11b29b006/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp

Labels: -Update-Monthly
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 8 2017

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

commit 952ae624c8001954d730c11e51fe8212725e878d
Author: Darren Shen <shend@chromium.org>
Date: Fri Dec 08 09:25:23 2017

[css-typed-om] Normalize numeric values.

This patch implements normalization of numeric values. Because we're
normalizing from property parsing and CSSOM, calc expressions are
simplified before they can be normalized. The spec is unclear about
whether this is acceptable.

See: https://github.com/w3c/css-houdini-drafts/issues/528

Spec: https://drafts.css-houdini.org/css-typed-om-1/#normalize-numeric

Bug:  788570 
Change-Id: I919372b01e2e629756546a49a8cbf3ecd15bd4ad
Reviewed-on: https://chromium-review.googlesource.com/812367
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522746}
[modify] https://crrev.com/952ae624c8001954d730c11e51fe8212725e878d/third_party/WebKit/LayoutTests/external/wpt/css/css-paint-api/registered-properties-in-custom-paint.https.html
[delete] https://crrev.com/245aa90ea20d593e9303b745e203996dbea2a37a/third_party/WebKit/LayoutTests/typedcssom/computedstyle/numbers-expected.txt
[delete] https://crrev.com/245aa90ea20d593e9303b745e203996dbea2a37a/third_party/WebKit/LayoutTests/typedcssom/inlinestyle/numbers-expected.txt
[add] https://crrev.com/952ae624c8001954d730c11e51fe8212725e878d/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-numeric.html
[modify] https://crrev.com/952ae624c8001954d730c11e51fe8212725e878d/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.cpp
[modify] https://crrev.com/952ae624c8001954d730c11e51fe8212725e878d/third_party/WebKit/Source/core/css/cssom/CSSPerspective.cpp
[modify] https://crrev.com/952ae624c8001954d730c11e51fe8212725e878d/third_party/WebKit/Source/core/css/cssom/CSSUnitValue.cpp

Cc: hs1217....@samsung.com shend@chromium.org
 Issue 798035  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 2 2018

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

commit ce65e32cc96765afe77a7388c1a2a876b37d593e
Author: Darren Shen <shend@chromium.org>
Date: Tue Jan 02 07:19:49 2018

[css-typed-om] Implement CSSPositionValue normalization.

Spec: https://drafts.css-houdini.org/css-typed-om-1/#positionvalue-normalization

Note that our implementation is much simpler than the spec algorithm
because the parser already handles a lot of the logic.

Bug:  788570 
Change-Id: Ice161237890222b6f28437a3846d8eb84bc0050c
Reviewed-on: https://chromium-review.googlesource.com/846579
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: meade_UTC10 <meade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526445}
[modify] https://crrev.com/ce65e32cc96765afe77a7388c1a2a876b37d593e/third_party/WebKit/LayoutTests/typedcssom/resources/testhelper.js
[add] https://crrev.com/ce65e32cc96765afe77a7388c1a2a876b37d593e/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/positionvalue-normalization.html
[delete] https://crrev.com/4853213e011c9f5b8bc67f4dd966a3213e71fd87/third_party/WebKit/LayoutTests/typedcssom/stylevalue-serialization/cssPositionValue.html
[modify] https://crrev.com/ce65e32cc96765afe77a7388c1a2a876b37d593e/third_party/WebKit/Source/core/css/cssom/CSSNumericValue.h
[modify] https://crrev.com/ce65e32cc96765afe77a7388c1a2a876b37d593e/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp
[modify] https://crrev.com/ce65e32cc96765afe77a7388c1a2a876b37d593e/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.h
[modify] https://crrev.com/ce65e32cc96765afe77a7388c1a2a876b37d593e/third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 3 2018

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

commit 8693043a12ca5efd8ef033b3d4937076c84dce32
Author: Darren Shen <shend@chromium.org>
Date: Wed Jan 03 07:09:06 2018

[css-typed-om] Add tests for normalizing resource values.

Adds tests for normalizing <url> and <gradient>s.

One test is failing because we can't normalize gradients yet.

Spec: https://drafts.css-houdini.org/css-typed-om-1/#resourcevalue-normalization

Bug:  788570 
Change-Id: I40d567a1e94c3ab4f9a3d7c8ca626da7661ac78e
Reviewed-on: https://chromium-review.googlesource.com/845244
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526643}
[modify] https://crrev.com/8693043a12ca5efd8ef033b3d4937076c84dce32/third_party/WebKit/LayoutTests/typedcssom/resources/testhelper.js
[add] https://crrev.com/8693043a12ca5efd8ef033b3d4937076c84dce32/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-resource-expected.txt
[add] https://crrev.com/8693043a12ca5efd8ef033b3d4937076c84dce32/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-resource.html
[modify] https://crrev.com/8693043a12ca5efd8ef033b3d4937076c84dce32/third_party/WebKit/LayoutTests/typedcssom/stylevalue-objects/parse-relative-url.html
[modify] https://crrev.com/8693043a12ca5efd8ef033b3d4937076c84dce32/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/cssUrlImageValue.html

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 9 2018

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

commit f492b885d3153686f1c9403aa4505cfc557bb510
Author: Darren Shen <shend@chromium.org>
Date: Tue Jan 09 08:03:16 2018

[css-typed-om] Fix <calc> normalization.

As resolved in:
https://github.com/w3c/css-houdini-drafts/issues/528

<calc>s are simplified before normalization for both CSSNumericValue.
parse and also CSSStyleValue.parse.

This patch reverts changes made in:
https://chromium.googlesource.com/chromium/src.git/+/2e87ecb8603b10fdbd2baa8f0c9d7048a9d9828f

as we will now always simplify. The number of tests also decrease as
we can no longer obtain complex CSSMathValues when we simplify.

Bug:  788570 
Change-Id: I8ae50a39911a887c18a4a229816ee76248942623
Reviewed-on: https://chromium-review.googlesource.com/851552
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527932}
[modify] https://crrev.com/f492b885d3153686f1c9403aa4505cfc557bb510/third_party/WebKit/LayoutTests/typedcssom/stylevalue-normalization/normalize-numeric.html
[modify] https://crrev.com/f492b885d3153686f1c9403aa4505cfc557bb510/third_party/WebKit/LayoutTests/typedcssom/stylevalue-subclasses/numeric-objects/parse.html
[modify] https://crrev.com/f492b885d3153686f1c9403aa4505cfc557bb510/third_party/WebKit/Source/core/css/CSSCalculationValue.cpp
[modify] https://crrev.com/f492b885d3153686f1c9403aa4505cfc557bb510/third_party/WebKit/Source/core/css/CSSCalculationValue.h
[modify] https://crrev.com/f492b885d3153686f1c9403aa4505cfc557bb510/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp

Comment 15 by shend@chromium.org, Jan 15 2018

Status: Fixed (was: Started)
Most of this is done. Will close this and file separate issues for any interop bugs we encounter.

Sign in to add a comment