Intent to Ship: Generated CSSStyleDeclaration Attributes

146 views
Skip to first unread message

Anders Hartvoll Ruud

unread,
Feb 15, 2021, 7:24:59 AM2/15/21
to blink-dev
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


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

unread,
Feb 18, 2021, 10:12:01 AM2/18/21
to Anders Hartvoll Ruud, blink-dev
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

unread,
Feb 18, 2021, 12:47:37 PM2/18/21
to Mike West, blink-dev
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

unread,
Feb 18, 2021, 3:30:24 PM2/18/21
to blink-dev, Anders Hartvoll Ruud, blink-dev, Mike West
From a pure architectural perspective, I'd like to see us pursue this (assuming the performance issues can be overcome).

LGTM1

Simon Pieters

unread,
Feb 18, 2021, 4:52:25 PM2/18/21
to Alex Russell, blink-dev, Anders Hartvoll Ruud, Mike West
Non-owner LGTM. Maybe you can give the WebKit folks a heads up about this change (after 3xLGTM) if they want to try again?



--
Simon Pieters

Anders Hartvoll Ruud

unread,
Feb 19, 2021, 5:23:12 AM2/19/21
to Simon Pieters, Alex Russell, blink-dev, Mike West
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

unread,
Feb 19, 2021, 5:47:12 AM2/19/21
to Anders Hartvoll Ruud, Simon Pieters, Alex Russell, blink-dev, Mike West

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

unread,
Feb 19, 2021, 6:00:56 AM2/19/21
to Manuel Rego Casasnovas, Simon Pieters, Alex Russell, blink-dev, Mike West
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

unread,
Feb 25, 2021, 5:19:37 AM2/25/21
to Manuel Rego Casasnovas, Simon Pieters, Alex Russell, blink-dev, Mike West
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

unread,
Feb 25, 2021, 7:03:26 AM2/25/21
to blink-dev, and...@chromium.org, si...@bocoup.com, sligh...@chromium.org, blink-dev, mk...@chromium.org, Manuel Rego
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

unread,
Feb 25, 2021, 7:24:36 AM2/25/21
to yo...@yoav.ws, blink-dev, si...@bocoup.com, sligh...@chromium.org, mk...@chromium.org, Manuel Rego
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

unread,
Feb 25, 2021, 10:31:03 AM2/25/21
to Anders Hartvoll Ruud, blink-dev, si...@bocoup.com, sligh...@chromium.org, mk...@chromium.org, Manuel Rego
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

unread,
Mar 1, 2021, 3:11:19 PM3/1/21
to Yoav Weiss, blink-dev, si...@bocoup.com, sligh...@chromium.org, mk...@chromium.org, Manuel Rego
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

unread,
Mar 2, 2021, 2:08:14 AM3/2/21
to Anders Hartvoll Ruud, Yoav Weiss, blink-dev, si...@bocoup.com, sligh...@chromium.org, Manuel Rego
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

unread,
Mar 2, 2021, 4:47:52 AM3/2/21
to Mike West, Anders Hartvoll Ruud, blink-dev, si...@bocoup.com, sligh...@chromium.org, Manuel Rego
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 :)
Reply all
Reply to author
Forward
0 new messages