CSS property aliases don't show up on CSSStyleDeclaration |
|||||||||
Issue descriptionChrome Version: 63.0.3218.0 What steps will reproduce the problem? (1) Object.getAllPropertyNames(document.body.style).indexOf(webkitTransform) What is the expected result? non-zero because "'webkitTransform' in document.body.style" is true, and "'webkitTransform' in CSSStyleDeclaration.prototype" is false. What happens instead? All property aliases seem to exist when looked up explicitly by name, but not when enumerating the properties on the object. I think property aliases are supposed to be an implementation detail, not web exposed as distinct from true properties, right? Eg. when we add an unprefixed 'foo' and switch 'webkitFoo' to be an alias of it, we don't want to have to worry that we're breaking some code somewhere which (for whatever reason) is depending on the keys it finds in style objects, right? But the main reason I care about this is that it causes problems for tools which try to introspect and see which CSS properties are supported, eg: https://github.com/GoogleChrome/confluence/issues/201. I seem to recall discussing this in a bug before (with Shane I think), but I can't find it anywhere at the moment (cc Shane in case he remembers).
,
Oct 3 2017
,
Oct 3 2017
,
Oct 3 2017
I think this was the original discussion (or one of the precursors, anyway) - https://groups.google.com/a/chromium.org/d/msgid/ecosystem-infra/CAHKdfMhD7XLM5p1BfXNe1APYM8_ik%3DmuEZTg7NBVn2zUODbggw%40mail.gmail.com?utm_medium=email&utm_source=footer
,
Oct 31 2017
,
Dec 6 2017
,
May 14 2018
futhark@, this has been sitting around for quite a long time and a workaround was required to discover support for (some) aliased properties in Web API Confluence: https://github.com/mdittmer/object-graph-js/pull/22 + https://github.com/mdittmer/web-apis/pull/60 Do you think this is an easy fix?
,
May 14 2018
It's not a five-minute patch as the serialization depends on looking up a specific CSSProperty class instance and get the string from that one. We don't have such classes for aliases atm. CC'ing andruud@ in case he wants to take a look. Btw, we have more issues in CSSStyleDeclaration enumeration. We include @-rule descriptors in style rules, and I think we list properties for @-rules using the CSSStyleDeclaration api.
,
May 14 2018
I think one way of fixing this might be in bug 700338 by generating attributes from CSSProperties.json5. I'm not sure how complex that would be to hook up to the current system, but it would provide a way to put aliases on the actual object.
,
May 14 2018
Generating a separate array of alias strings from the json5 file, adding an extra for-loop to add those in NamedPropertyEnumerator would probably be the simplest solution, yes. You'd probably want the runtime flag check that the CSSProperty classes have via IsEnabled(), though.
,
May 15 2018
,
May 15 2018
We should probably also enumerate the dashed properties?
,
May 15 2018
I think so, yes, per https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-_dashed_attribute
,
May 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aae947a01acaf7bce18c309244b64c8ef110a349 commit aae947a01acaf7bce18c309244b64c8ef110a349 Author: Anders Hartvoll Ruud <andruud@chromium.org> Date: Fri May 18 09:43:15 2018 Enumerate aliases on CSSPropertyDeclaration. Currently, property aliases do not appear when enumerating CSSPropertyDeclaration. We generate CSSUnresolvedProperty-subclasses for alias properties, but these classes do not provide a GetJSPropertyName function, hence CSSPropertyDeclaration:: NamedPropertyEnumerator can't currently know which property names to write. This patch lifts CSSProperty::GetJSPropertyName/IsEnabled to CSSUnresolvedProperty and adds generation of these functions to alias properties. Notes on runtime flags: * The runtime_flag setting on aliased properties already propagate to the alias. (See css_properties.py:expand_aliases). * It turns out that we currently have no aliased property with a runtime flag. This means IsEnabled-generation is currently unused for aliases. I have manually tested a fake alias against a runtime_flag'd property to verify that the generated code works. R=foolip@chromium.org, futhark@chromium.org Bug: 768917 Change-Id: Ibde70b71416bdbe792d05cc21f31794f997cb968 Reviewed-on: https://chromium-review.googlesource.com/1059615 Commit-Queue: Anders Ruud <andruud@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#559848} [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/animations/animations-parsing-001.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/animations/animations-parsing-002.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/animations/animations-parsing-003.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/animations/animations-parsing-004.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/animations/animations-parsing-005.html [add] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/external/wpt/compat/css-style-declaration-alias-enumeration.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/transitions/transitions-parsing-001.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/transitions/transitions-parsing-002.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/transitions/transitions-parsing-003.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/transitions/transitions-parsing-004.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/transitions/transitions-parsing-005.html [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/virtual/stable/webexposed/css-properties-as-js-properties-expected.txt [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/WebKit/LayoutTests/webexposed/css-properties-as-js-properties-expected.txt [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/blink/renderer/build/scripts/core/css/properties/make_css_property_subclasses.py [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/blink/renderer/build/scripts/core/css/properties/templates/css_property.h.tmpl [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/blink/renderer/build/scripts/core/css/properties/templates/css_property_subclass.h.tmpl [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/blink/renderer/build/scripts/core/css/properties/templates/css_unresolved_property.h.tmpl [modify] https://crrev.com/aae947a01acaf7bce18c309244b64c8ef110a349/third_party/blink/renderer/core/css/css_style_declaration.cc
,
May 28 2018
,
May 31 2018
Thank you andrudd@, this is great :) |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by rbyers@chromium.org
, Sep 26 2017FWIW, Safari does something similar, but aliased properties do show up on the prototype chain in Firefox and Edge: Object.getOwnPropertyNames(Object.getPrototypeOf(document.body.style)).indexOf('webkitTransform') > 783 Object.getOwnPropertyNames(Object.getPrototypeOf(document.body.style)).indexOf('transform') > 607 I don't know if any of this is spec'd, but regardless we should aim to be interoperable and unsurprising. I can see an argument for making the non-enumerable (even though they are enumerable today in Firefox and Edge), but IMHO it's pretty surprising that they don't show up at all in the OwnProperties list. I'm less concerned about the weird dashed-names (document.body.style['-webkit-transform']), although there are interop differences with how those are exposed across browser too.