New issue
Advanced search Search tips

Issue 812915 link

Starred by 3 users

Issue metadata

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

Blocking:
issue 545318



Sign in to add a comment

[css-typed-om] Ensure spec IDL matches Blink IDLs

Project Member Reported by shend@chromium.org, Feb 16 2018

Issue description

Comment 1 by shend@chromium.org, Feb 20 2018

Owner: shend@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 22 2018

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

commit dfb7271a0d6ecc467597ef77b71092da26e369cc
Author: Darren Shen <shend@chromium.org>
Date: Thu Feb 22 00:30:56 2018

[css-typed-om] Ensure CSSStyleValue IDL matches spec.

The only difference is that parse/parseAll should never return null
(either return non-null or throw an error).

Bug:  812915 
Change-Id: I4de3c68d17786f8fdbaf53b02ba5ec05b8efc45f
Reviewed-on: https://chromium-review.googlesource.com/930061
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538278}
[modify] https://crrev.com/dfb7271a0d6ecc467597ef77b71092da26e369cc/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.cpp
[modify] https://crrev.com/dfb7271a0d6ecc467597ef77b71092da26e369cc/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.h
[modify] https://crrev.com/dfb7271a0d6ecc467597ef77b71092da26e369cc/third_party/WebKit/Source/core/css/cssom/CSSStyleValue.idl

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 22 2018

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

commit 43700726a48ebedf10af437c644ec46d862fb46b
Author: Darren Shen <shend@chromium.org>
Date: Thu Feb 22 01:44:17 2018

[css-typed-om] Ensure StylePropertyMapReadonly IDL matches spec.

Spec added a new .size member, which we implement in this patch.

Bug:  812915 
Change-Id: Ib48fc7b8053c41e4c79dc9cc280bc18376e5fbe0
Reviewed-on: https://chromium-review.googlesource.com/930101
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538304}
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/interfaces-expected.txt
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/LayoutTests/external/wpt/interfaces/css-typed-om.idl
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/declared/delete-rule-crash.html
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/DeclaredStylePropertyMap.cpp
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/DeclaredStylePropertyMap.h
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.h
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.h
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.h
[modify] https://crrev.com/43700726a48ebedf10af437c644ec46d862fb46b/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.idl

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 22 2018

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

commit 35bfb10373c5b34c193ad9a7c569d6aa429028fc
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Thu Feb 22 01:56:40 2018

Revert "[css-typed-om] Ensure StylePropertyMapReadonly IDL matches spec."

This reverts commit 43700726a48ebedf10af437c644ec46d862fb46b.

Reason for revert: Causes build failure on Google Chrome ChromeOS:
https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20ChromeOS/45929

Original change's description:
> [css-typed-om] Ensure StylePropertyMapReadonly IDL matches spec.
> 
> Spec added a new .size member, which we implement in this patch.
> 
> Bug:  812915 
> Change-Id: Ib48fc7b8053c41e4c79dc9cc280bc18376e5fbe0
> Reviewed-on: https://chromium-review.googlesource.com/930101
> Reviewed-by: nainar <nainar@chromium.org>
> Commit-Queue: Darren Shen <shend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538304}

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

Change-Id: I442aebc331ddc97c09c870299ab0ab76f5ce7773
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  812915 
Reviewed-on: https://chromium-review.googlesource.com/930179
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538309}
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/interfaces-expected.txt
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/LayoutTests/external/wpt/interfaces/css-typed-om.idl
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/declared/delete-rule-crash.html
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/DeclaredStylePropertyMap.cpp
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/DeclaredStylePropertyMap.h
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.h
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.h
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.h
[modify] https://crrev.com/35bfb10373c5b34c193ad9a7c569d6aa429028fc/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.idl

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2018

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

commit ba83106f73ba84d1f7f11aa1c57d93da2b44ed45
Author: Darren Shen <shend@chromium.org>
Date: Thu Feb 22 02:20:14 2018

Revert "Revert "[css-typed-om] Ensure StylePropertyMapReadonly IDL matches spec.""

This reverts commit 35bfb10373c5b34c193ad9a7c569d6aa429028fc.

Reason for revert: Trying to reland.

Original change's description:
> Revert "[css-typed-om] Ensure StylePropertyMapReadonly IDL matches spec."
>
> This reverts commit 43700726a48ebedf10af437c644ec46d862fb46b.
>
> Reason for revert: Causes build failure on Google Chrome ChromeOS:
> https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20ChromeOS/45929
>
> Original change's description:
> > [css-typed-om] Ensure StylePropertyMapReadonly IDL matches spec.
> >
> > Spec added a new .size member, which we implement in this patch.
> >
> > Bug:  812915 
> > Change-Id: Ib48fc7b8053c41e4c79dc9cc280bc18376e5fbe0
> > Reviewed-on: https://chromium-review.googlesource.com/930101
> > Reviewed-by: nainar <nainar@chromium.org>
> > Commit-Queue: Darren Shen <shend@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#538304}
>
> TBR=nainar@chromium.org,shend@chromium.org
>
> Change-Id: I442aebc331ddc97c09c870299ab0ab76f5ce7773
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  812915 
> Reviewed-on: https://chromium-review.googlesource.com/930179
> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538309}

TBR=nainar@chromium.org

Bug:  812915 
Change-Id: Ia45bc81d58a807448e00d21a33c97e2210712bb2
Reviewed-on: https://chromium-review.googlesource.com/930521
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538321}
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/interfaces-expected.txt
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/LayoutTests/external/wpt/interfaces/css-typed-om.idl
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/LayoutTests/typedcssom/the-stylepropertymap/declared/delete-rule-crash.html
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.h
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/DeclaredStylePropertyMap.cpp
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/DeclaredStylePropertyMap.h
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.h
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.h
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.h
[modify] https://crrev.com/ba83106f73ba84d1f7f11aa1c57d93da2b44ed45/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.idl

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 22 2018

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

commit 692b8e57542917a11c5c6c5fb62a9787925e8ec7
Author: Darren Shen <shend@chromium.org>
Date: Thu Feb 22 06:36:00 2018

[css-typed-om] Make sure implementation IDL matches spec IDL.

This patch updates our IDL to match that of the spec. We found two
issues in the process:
- StylePropertyMap.get should return (undefined or CSSStyleValue),
  but IDL doesn't provide that yet. The spec uses "any" as a workaround
  but we'll return (null or CSSStyleValue) for now (null vs undefined).
  We'll change the code once IDL gives us something for this.
- Iteration should always return a sequence of CSSStyleValue, whereas
  they used to return either CSSStyleValue or sequence of CSSStyleValues
  depending on whether the property was list valued or not.

We change the code to fix this.

We deleted the inlineStylePropertyMap_iteration.html LayoutTest
as it is already covered by WPT.

We also update the WPT test expectations to match the spec.

Bug:  812915 
Change-Id: Id2f9fbba4398c9ecfd63ea3f8e1aad0b3a0b499f
Reviewed-on: https://chromium-review.googlesource.com/925981
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538367}
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/computed/iterable.tentative.html
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/declared/iterable.tentative.html
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/the-stylepropertymap/inline/iterable.tentative.html
[delete] https://crrev.com/1023ea047908163438fc9d986a09fa2066347019/third_party/WebKit/LayoutTests/typedcssom/inlinestyle/inlineStylePropertyMap_iteration.html
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/LayoutTests/typedcssom/inlinestyle/inlineStylePropertyMap_iterationWithModification.html
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.cpp
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.h
[modify] https://crrev.com/692b8e57542917a11c5c6c5fb62a9787925e8ec7/third_party/WebKit/Source/core/css/cssom/StylePropertyMapReadOnly.idl

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 22 2018

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

commit 0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3
Author: Darren Shen <shend@chromium.org>
Date: Thu Feb 22 07:19:24 2018

[css-typed-om] Update WPT IDL test and clean up implementation IDL.

This patch updates the WPT IDL test to the current spec.
We also clean up the implementation IDL to match the spec.

Bug:  812915 
Change-Id: Id2ca308832344e824b0e7b7eaa6198dc6d7c7e03
Reviewed-on: https://chromium-review.googlesource.com/930502
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538371}
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/LayoutTests/external/wpt/css/css-typed-om/interfaces-expected.txt
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/LayoutTests/external/wpt/interfaces/css-typed-om.idl
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/Source/core/css/cssom/CSSNumericArray.idl
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.idl
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/Source/core/css/cssom/CSSUnitValues.idl
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/Source/core/css/cssom/StylePropertyMap.h
[modify] https://crrev.com/0480e7b9c70d52c2bbfb54717ceecbfe4d6e1cf3/third_party/WebKit/Source/core/css/cssom/StylePropertyMap.idl

Comment 8 by shend@chromium.org, Feb 25 2018

I think only changing DOMString -> USVString is remaining, but there are some discussions still happening:
https://github.com/w3c/css-houdini-drafts/issues/687

Comment 9 by shend@chromium.org, Apr 18 2018

Status: Fixed (was: Started)
CSSWG resolved to use CSSOMStrings: https://github.com/w3c/css-houdini-drafts/issues/687#issuecomment-379669504

Will close this bug and open a separate one for this.

Sign in to add a comment