Intent to Ship: Generated CSSStyleDeclaration Attributes

[Anders Hartvoll Ruud]

Contact emails
and...@chromium.org

Summary
This I2S has two web-facing effects: 

1) Properties are exposed via CSSStyleDeclaration's prototype

Blink currently exposes CSS properties on CSSStyleDeclaration (typically seen as style.color, style.fontSize, etc) using named properties. This is wrong, and causes those attributes to appear as the object's "own" properties. Instead, the spec requires all properties to appear as normal attributes on CSSStyleDeclaration (see spec around and near camel-cased attributes), which means that they'll be exposed via CSSStyleDeclaration's prototype instead.

Due to the large number of CSS properties (500+), and since many attributes appear more than once (due to different casing), binary size is an issue here, and the initial version of this fix increased the Android binary size by 326Kb (!). Thankfully the bindings team (yukishiino@) helped out here with a special extended attribute, which reduced the binary impact to "only" 32Kb.

2) Dashed attributes and webkit-cased attributes are now visible when enumerating attributes

See diff for css-properties-as-js-properties-expected.txt in the CL.

Blink component
Blink>CSS

TAG review status
Not applicable

Risks

Interoperability and Compatibility

This change is not without risks, since code that interacts with properties on CSSStyleDeclaration is now subtly different. For example, code that relies on Object.defineProperty to set style attributes will "silently" no longer work. (See the app_view.js diff for an example where this broke our own JS).

This risk is mitigated by the fact that anything that is broken by this change is already broken in Firefox, so any (new) breakage should be limited to browser-sniffing cases or Chrome-specific code.

Unfortunately there is no (known) way of adding a use-counter or otherwise estimating the breakage in this case.

Gecko: Shipped/Shipping

WebKit: Made the same change in October last year: https://trac.webkit.org/changeset/268564/webkit

Unfortunately the change was later reverted due to binary size issues. However, this shows that the intent to change is there.

Web developers: No signals

Is this feature fully tested by web-platform-tests?
Yes

Tracking bug
https://bugs.chromium.org/p/chromium/issues/detail?id=700338

Link to entry on the Chrome Platform Status
https://chromestatus.com/feature/6464391237074944

This intent message was generated by Chrome Platform Status.

[Mike West]

In principle, this seems like the right thing to do.

Practically, given that WebKit made the change, reverted it due to binary size increases, and hasn't tried again in the last ~4 months, and that we found breakage in the past that relied on Chrome's behavior: did you consider making the wrong thing the right thing by asking Gecko to align with WebKit and Blink? How terrible would it be if we did that instead?

-mike

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKFBnUq4JLkos4z5EfbNfOc%2BHbRgjaYHrC-ENRaoJi34GtX7dA%40mail.gmail.com.

[Anders Hartvoll Ruud]

On Thu, Feb 18, 2021 at 4:11 PM Mike West <mk...@chromium.org> wrote:

In principle, this seems like the right thing to do.

Practically, given that WebKit made the change, reverted it due to binary size increases, and hasn't tried again in the last ~4 months, and that we found breakage in the past that relied on Chrome's behavior: did you consider making the wrong thing the right thing by asking Gecko to align with WebKit and Blink? How terrible would it be if we did that instead?

-mike

Ah yes, good point. I have not attempted to go this direction (again), because foolip@ already suggested this and after some discussion that proposal was closed as a WontFix. For relevant details, see bzbarsky's "usual list of reasons why we don't just use 'getter' for everything".

[Alex Russell]

From a pure architectural perspective, I'd like to see us pursue this (assuming the performance issues can be overcome).

LGTM1

[Simon Pieters]

Non-owner LGTM. Maybe you can give the WebKit folks a heads up about this change (after 3xLGTM) if they want to try again?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/f7aae244-c140-4f34-828a-d35aa79ffa47n%40chromium.org.

--

Simon Pieters

Bocoup https://bocoup.com/

[Anders Hartvoll Ruud]

On Thu, Feb 18, 2021 at 10:52 PM Simon Pieters <si...@bocoup.com> wrote:

Non-owner LGTM. Maybe you can give the WebKit folks a heads up about this change (after 3xLGTM) if they want to try again?

Sure, I'll do that. 

 

On Thu, Feb 18, 2021 at 9:30 PM Alex Russell <sligh...@chromium.org> wrote:

From a pure architectural perspective, I'd like to see us pursue this (assuming the performance issues can be overcome).

* Ahem *

Which performance issues are you referring to?

[Manuel Rego Casasnovas]

On 19/02/2021 11:22, Anders Hartvoll Ruud wrote:
> On Thu, Feb 18, 2021 at 9:30 PM Alex Russell
> <sligh...@chromium.org <mailto:sligh...@chromium.org>> wrote:
>
> From a pure architectural perspective, I'd like to see us
> pursue this (assuming the performance issues can be overcome).
>
>
> * Ahem *
>
> Which performance issues are you referring to?

WebKit had to revert the change due to binary size issues, would that be
an issue in Chromium or not?

Bye,
Rego

[Anders Hartvoll Ruud]

Yes, this would have been an issue for Blink: the initial version of the fix (which never landed) inflated the binary size with ~326kB. We didn't find that very acceptable, so the bindings team / yukishiino@ implemented a special optimization for this use-case which brought the number down to the current ~32kB.

 

Bye,
   Rego

[Anders Hartvoll Ruud]

To summarize on why this change is good:

• The current interop situation is not great, and a real pain point for authors:
• Comment
• Article excerpt: [Getting a list of all CSS properties from the browser: ] In Chrome and Safari, this is as simple as Object.getOwnPropertyNames(document.body.style). However, in Firefox, this doesn’t work. Why is that? To understand this (and how to work around it), we need to dig a bit deeper". (Pretty sad that it is necessary to write an article about this).
• bzbarsky lists several good points for why this change is better than other vendors aligning with our current behavior. Perhaps the most important reason from the author's perspective is that "it would break the ability of webpages to hook or polyfill the getter/setter for a given property". (I assume this is because the own-property "slot" is already occupied in our implementation).

[yo...@yoav.ws]

On Thursday, February 25, 2021 at 11:19:37 AM UTC+1 and...@chromium.org wrote:

To summarize on why this change is good:

• The current interop situation is not great, and a real pain point for authors:
  • Comment
  • Article excerpt: [Getting a list of all CSS properties from the browser: ] In Chrome and Safari, this is as simple as Object.getOwnPropertyNames(document.body.style). However, in Firefox, this doesn’t work. Why is that? To understand this (and how to work around it), we need to dig a bit deeper". (Pretty sad that it is necessary to write an article about this).

Doesn't this support the opposite view? (that the current Chromium implementation is simpler to use) 

[Anders Hartvoll Ruud]

On Thu, Feb 25, 2021 at 1:03 PM yo...@yoav.ws <yo...@yoav.ws> wrote:

On Thursday, February 25, 2021 at 11:19:37 AM UTC+1 and...@chromium.org wrote:

To summarize on why this change is good:

• The current interop situation is not great, and a real pain point for authors:
  • Comment
  • Article excerpt: [Getting a list of all CSS properties from the browser: ] In Chrome and Safari, this is as simple as Object.getOwnPropertyNames(document.body.style). However, in Firefox, this doesn’t work. Why is that? To understand this (and how to work around it), we need to dig a bit deeper". (Pretty sad that it is necessary to write an article about this).

Doesn't this support the opposite view? (that the current Chromium implementation is simpler to use) 

(Ah, that's unfortunate, I didn't see that interpretation).

No, I don't think this is support for the current way. I think "simple" is used here in a trivial way because Object.getOwnPropertyNames(document.body.style) is shorter than Object.getOwnPropertyNames(Object.getPrototypeOf(document.body.style)).

The important part is that it doesn't work the same way across browsers.

[Yoav Weiss]

On Thu, Feb 25, 2021 at 1:24 PM Anders Hartvoll Ruud <and...@chromium.org> wrote:

On Thu, Feb 25, 2021 at 1:03 PM yo...@yoav.ws <yo...@yoav.ws> wrote:

On Thursday, February 25, 2021 at 11:19:37 AM UTC+1 and...@chromium.org wrote:

To summarize on why this change is good:

• The current interop situation is not great, and a real pain point for authors:
  • Comment
  • Article excerpt: [Getting a list of all CSS properties from the browser: ] In Chrome and Safari, this is as simple as Object.getOwnPropertyNames(document.body.style). However, in Firefox, this doesn’t work. Why is that? To understand this (and how to work around it), we need to dig a bit deeper". (Pretty sad that it is necessary to write an article about this).

Doesn't this support the opposite view? (that the current Chromium implementation is simpler to use) 

(Ah, that's unfortunate, I didn't see that interpretation).

No, I don't think this is support for the current way. I think "simple" is used here in a trivial way because Object.getOwnPropertyNames(document.body.style) is shorter than Object.getOwnPropertyNames(Object.getPrototypeOf(document.body.style)).

Thanks for outlining the differences! Both indeed seem simple enough.

The question I have in the back of my mind on this is "Will WebKit follow?"

Might be worthwhile to ask them directly (and maybe point them to the binding magic the team did to reduce the binary size impact)

[Anders Hartvoll Ruud]

On Thu, Feb 25, 2021 at 4:30 PM Yoav Weiss <yo...@yoav.ws> wrote:

On Thu, Feb 25, 2021 at 1:24 PM Anders Hartvoll Ruud <and...@chromium.org> wrote:

On Thu, Feb 25, 2021 at 1:03 PM yo...@yoav.ws <yo...@yoav.ws> wrote:

On Thursday, February 25, 2021 at 11:19:37 AM UTC+1 and...@chromium.org wrote:

To summarize on why this change is good:

• The current interop situation is not great, and a real pain point for authors:
  • Comment
  • Article excerpt: [Getting a list of all CSS properties from the browser: ] In Chrome and Safari, this is as simple as Object.getOwnPropertyNames(document.body.style). However, in Firefox, this doesn’t work. Why is that? To understand this (and how to work around it), we need to dig a bit deeper". (Pretty sad that it is necessary to write an article about this).

Doesn't this support the opposite view? (that the current Chromium implementation is simpler to use) 

(Ah, that's unfortunate, I didn't see that interpretation).

No, I don't think this is support for the current way. I think "simple" is used here in a trivial way because Object.getOwnPropertyNames(document.body.style) is shorter than Object.getOwnPropertyNames(Object.getPrototypeOf(document.body.style)).

Thanks for outlining the differences! Both indeed seem simple enough.

The question I have in the back of my mind on this is "Will WebKit follow?"

Might be worthwhile to ask them directly (and maybe point them to the binding magic the team did to reduce the binary size impact)

I got a confirmation from weinig@apple that they still intend to make the change: (Slack messages included inline for the lazy):

andruud: ... <omitted: summary of situation> ...
andruud: However, since you reverted your change, the question is ... do you still intend to make the change?
weinig: Yeah, I’d still like to make the change in some form. When I get some time, I’ll try to come up with another strategy.
andruud: @weinig :thumbsup: Is it OK if I include your comment in the relevant intent-to-ship on the blink-dev mailing list?
weinig: sure
weinig: I filed https://bugs.webkit.org/show_bug.cgi?id=222518 this weekend to track reducing our compile time / binary size issues so we can re-enable it

Source: https://webkit.slack.com/archives/CU64U6FDW/p1614335312067700

I did offer to point to our extended attribute solution to the binary size problem, but they didn't take me up on that (yet).

[Mike West]

LGTM2.

I agree with Alex that this is the right direction, and if WebKit is going to try again, it's worth shipping it ourselves. 

-mike

[Yoav Weiss]

LGTM3

On Tue, Mar 2, 2021 at 8:08 AM Mike West <mk...@chromium.org> wrote:

LGTM2.

I agree with Alex that this is the right direction, and if WebKit is going to try again, it's worth shipping it ourselves. 

-mike

On Mon, Mar 1, 2021 at 9:11 PM Anders Hartvoll Ruud <and...@chromium.org> wrote:

On Thu, Feb 25, 2021 at 4:30 PM Yoav Weiss <yo...@yoav.ws> wrote:

On Thu, Feb 25, 2021 at 1:24 PM Anders Hartvoll Ruud <and...@chromium.org> wrote:

On Thu, Feb 25, 2021 at 1:03 PM yo...@yoav.ws <yo...@yoav.ws> wrote:

On Thursday, February 25, 2021 at 11:19:37 AM UTC+1 and...@chromium.org wrote:

To summarize on why this change is good:

• The current interop situation is not great, and a real pain point for authors:
  • Comment
  • Article excerpt: [Getting a list of all CSS properties from the browser: ] In Chrome and Safari, this is as simple as Object.getOwnPropertyNames(document.body.style). However, in Firefox, this doesn’t work. Why is that? To understand this (and how to work around it), we need to dig a bit deeper". (Pretty sad that it is necessary to write an article about this).

Doesn't this support the opposite view? (that the current Chromium implementation is simpler to use) 

(Ah, that's unfortunate, I didn't see that interpretation).

No, I don't think this is support for the current way. I think "simple" is used here in a trivial way because Object.getOwnPropertyNames(document.body.style) is shorter than Object.getOwnPropertyNames(Object.getPrototypeOf(document.body.style)).

Thanks for outlining the differences! Both indeed seem simple enough.

The question I have in the back of my mind on this is "Will WebKit follow?"

Might be worthwhile to ask them directly (and maybe point them to the binding magic the team did to reduce the binary size impact)

I got a confirmation from weinig@apple that they still intend to make the change: (Slack messages included inline for the lazy):

Thanks for inlining not only for the lazy, but also for those without an account on that Slack :)