Closed Bug 1246796 Opened 4 years ago Closed 3 years ago
Add support for elm
.style['-webkit-foo'] style CSSOM getters/setters
62.46 KB, image/png
1.55 KB, patch
|Details | Diff | Splinter Review|
2.08 KB, patch
|Details | Diff | Splinter Review|
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.
Boris, assuming we feel like we actually need to do this, would there be much more needed than this patch (other than tests)?
Well, and fixes to the comment above, and updates to the relevant specs, right? ;) As far as actual code changes, no.
(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.
AFAICT this should be covered in CSSOM by "dashed attribute". e.style['-moz-appearance'] should also work but doesn't currently.
I suppose making the patch do this instead would more closely match the spec and other browsers: - if prop != name and name != "-": + if prop != name:
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.
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.
David, thoughts on whether we should just expose -moz stuff here?
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.
Seems Edge is happy to allow elm.style['-ms-blah'] setting too.
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)
Seems there's a framework Marfeel using this style of setter: https://bugzilla.mozilla.org/show_bug.cgi?id=1149160#c2
I guess we should just give up and ship this crap.
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 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.
Attachment #8773344 - Flags: review?(bzbarsky) → review+
Cool, will do.
Attachment #8773344 - Attachment is obsolete: true
Attachment #8717200 - Attachment is obsolete: true
Oranges look truly intermittant, let's push.
Pushed by firstname.lastname@example.org: 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
(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.
You need to log in before you can comment on or make changes to this bug.