1246796 - Add support for elm.style['-webkit-foo'] style CSSOM getters/setters

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Mike Taylor [:miketaylr]]

As mentioned in https://github.com/webcompat/web-bugs/issues/1039#issuecomment-181595677, Edge, Blink and WebKit all support something like a "webkit prefixed dashed attribute."

I have a simple patch that hacks support in and confirm that it does in fact make the broken carousel work as expected. 

One thing to note, I don't know if it's possible to make CSS2Properties non-enumerable or whatever, but it seems like WebKit/Blink/Edge all have done that, compared to my local build: https://cloudup.com/c-m8MSDWT3c.

(It may be that we implement a newer version of the WebIDL spec and that's expected, I don't know. Looks kinda ugly.)

I think this is the first instance of this kind of bustage we've seen -- I'd like to try to find more data here.

Comment 1

[Mike Taylor [:miketaylr]]

Attachment: Add support for something like a "webkit prefixed dashed attribute."

Comment 2

[Mike Taylor [:miketaylr]]

Boris, assuming we feel like we actually need to do this, would there be much more needed than this patch (other than tests)?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well, and fixes to the comment above, and updates to the relevant specs, right?  ;)

As far as actual code changes, no.

Comment 4

[Mike Taylor [:miketaylr]]

(In reply to Boris Zbarsky [:bz] from comment #3)
> Well, and fixes to the comment above, and updates to the relevant specs,
> right?  ;)

Heh, yeah. Let me do some crawling of existing compat issues and JS corpora to see how much we actually need this and we'll go from there.

Comment 5

[Simon Pieters [:zcorpan]]

AFAICT this should be covered in CSSOM by "dashed attribute". e.style['-moz-appearance'] should also work but doesn't currently.

Comment 6

[Simon Pieters [:zcorpan]]

I suppose making the patch do this instead would more closely match the spec and other browsers:

-    if prop != name and name[0] != "-":
+    if prop != name:

Comment 7

[Boris Zbarsky [:bzbarsky]]

Hmm.  We explicitly exclude vendor-prefixed stuff from the "dashed attribute" thing, because it's the right thing to do and because it's what the spec says to do.  I guess we could stop doing that, but it seems really bizarre to shit prefixed stuff all over the web like that.  :(

In terms of the spec, as I said the spec does NOT include prefixed stuff in its dashed attribute thing.  The spec says:

  For each CSS property property that is a supported CSS property, except
  for properties that have no "-" (U+002D) in the property name, the
  following partial interface applies where dashed attribute is property.

where "supported CSS property" is defined as:

  The term supported CSS property refers to a CSS property that the user
  agent implements, and that is defined to be a case-insensitive property
  in the CSS specification. A supported CSS property must be in its lowercase
  form for the purpose of comparisons this specification.

Prefixed stuff fulfills the "user agent implements" condition, but fails the "defined in the CSS specification" condition.

Comment 8

[Simon Pieters [:zcorpan]]

OK, that was not what I intended the spec to require. Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=29440

I suppose I could make the spec to exclude vendor-specific properties other than -webkit- ones, if you prefer that, though I'd like to check what Edge does for -ms-foo also.

Comment 9

[Boris Zbarsky [:bzbarsky]]

David, thoughts on whether we should just expose -moz stuff here?

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If we're going to expose unprefixed properties and -webkit- properties, we should probably do -moz- ones as well, just because developers will assume that things are consistent.

Comment 11

[Mike Taylor [:miketaylr]]

Attachment: Edge-prefixed-dashed-attribute.png

Seems Edge is happy to allow elm.style['-ms-blah'] setting too.

Comment 12

[Mike Taylor [:miketaylr]]

I re-wrote my top Alexa JS crawler to "beautify" and inline JS and let it run last night -- it only got 16071 domains because the battery died (oops), but here's the results from that:

https://gist.github.com/miketaylr/a014b68938c31619f8c4

671 tokens in Alexa Top 16071 (but note that token != site)

Comment 13

[Mike Taylor [:miketaylr]]

Seems there's a framework Marfeel using this style of setter: https://bugzilla.mozilla.org/show_bug.cgi?id=1149160#c2

Comment 14

[Boris Zbarsky [:bzbarsky]]

I guess we should just give up and ship this crap.

Comment 15

[Mike Taylor [:miketaylr]]

Attachment: Allow getting/setting dashed attributes that start with "-", for web compatibility. r?bz

Comment 16

[Mike Taylor [:miketaylr]]

I changed the comments here to imply the spec is explicit about this change. Can we get https://www.w3.org/Bugs/Public/show_bug.cgi?id=29440 fixed accordingly?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8773344 [details] [diff] [review]
Allow getting/setting dashed attributes that start with "-", for web compatibility. r?bz

r=me, but please add tests.

Comment 18

[Mike Taylor [:miketaylr]]

Cool, will do.

Comment 19

[Mike Taylor [:miketaylr]]

Attachment: Update test for CSS prop setters/getters.

Comment 20

[Mike Taylor [:miketaylr]]

Attachment: Allow getting/setting dashed attributes that start with "-", for web compatibility.

Comment 21

[Mike Taylor [:miketaylr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bed8370f5104

Comment 22

[Mike Taylor [:miketaylr]]

Oranges look truly intermittant, let's push.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2a6a2af5243
Allow getting/setting dashed attributes that start with "-", for web compatibility. r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/7df92788ffeb
Update test for CSS prop setters/getters. r=me

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f2a6a2af5243
https://hg.mozilla.org/mozilla-central/rev/7df92788ffeb

Comment 25

[Simon Pieters [:zcorpan]]

(In reply to Mike Taylor [:miketaylr] from comment #16)
> I changed the comments here to imply the spec is explicit about this change.
> Can we get https://www.w3.org/Bugs/Public/show_bug.cgi?id=29440 fixed
> accordingly?

Now fixed.

Comment 26

[Mike Taylor [:miketaylr]]

Thanks!