958887 - Add support for element.style["css-property-name"] non-standard extension

Status: RESOLVED FIXED

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

Description

[rennie.degraaf]

Attachment: Test case

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131210132650

Steps to reproduce:

Changing the CSS property background-image via JavaScript does not seem to work in firefox 26.0. It does work in Chromium 27.

The attached test case sets the background-image of a div to a red X, then changes it to a green checkmark using a script. Loading it in Chromium shows the green checkmark; loading it in Firefox shows the red x indicating that the script had no effect. The images used in the test case are SVG, but I got the same result with PNG images.  

Setting the background property, rather than the background-image property, works.


Actual results:

Background image did not change.


Expected results:

Background image should have changed.

Comment 1

[Dave Garrett]

Attachment: with style.backgroundImage

https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Properties_Reference

Dashes aren't viable for JavaScript property names, so lots of them are different from the name in CSS. Generally, the dashes are dropped and the names are switched to camel-case. The "background-image" JS property is "backgroundImage". I'm surprised style["background-image"] instead of style.backgroundImage works in Chromium.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Still invalid per <http://dev.w3.org/csswg/cssom/#cssstyledeclaration>, it seems.

Comment 3

[Alice0775 White]

related to Bug 947887, Bug 573880 ?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> It does work in Chromium 27.

See http://lists.w3.org/Archives/Public/www-style/2012Feb/0655.html but basically Trident, WebKit, and Blink have a non-standard extensions where in addition to the standardized properties on CSSDeclaration they also implement a bunch of other properties.  Gecko and Presto don't do the non-standard bits here.

Of course the only response to my mail was sidetracking by Mr. Adams, about as expected, and in particular no response from anyone at Apple, Google, or Microsoft.  So maybe we should just give up, also implement this non-standard extension, and wait for the spec to catch up.

Comment 5

[rennie.degraaf]

Oh.  I guess I don't adequately understand the difference between foo.style[prop] and foo.style.getPropertyValue(prop).

Sorry for the entropy, and keep up the good work!

Comment 6

[Simon Pieters [:zcorpan]]

Also see https://code.google.com/p/chromium/issues/detail?id=290055

Comment 7

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

IIRC, WebKit/Blink behavior is also case-insensitive, at least in some places (I think for both the hyphenated and camelCased names, though maybe only the camelCased ones).

Comment 8

[Simon Pieters [:zcorpan]]

It's not case-insensitive as far as I can tell. http://software.hixie.ch/utilities/js/live-dom-viewer/saved/2732

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Also see https://code.google.com/p/chromium/issues/detail?id=290055

Which is basically being ignored as far as I can tell, right?  :(

Comment 10

[Boris Zbarsky [:bzbarsky]]

The main spec issue here, btw, is whether these properties live on the prototype or on the object itself....

Comment 11

[Dave Garrett]

I think there's at least enough desire to mark this bug as an RFE to consider adding this. The JS prop names have always been mildly annoying/confusing and this extension does make some sense.

(In reply to Boris Zbarsky [:bz] from comment #4)
> So maybe we should just give up, also implement this
> non-standard extension, and wait for the spec to catch up.

Is there a bug filed somewhere to add this to spec?

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Is there a bug filed somewhere to add this to spec?

I haven't filed one.

Comment 13

[Simon Pieters [:zcorpan]]

(In reply to Boris Zbarsky [:bz] from comment #9)
> Which is basically being ignored as far as I can tell, right?  :(

No, it just takes some time to get data from the use counter. It should be available now I think, I've asked davve to have a look.

(In reply to Dave Garrett from comment #11)
> Is there a bug filed somewhere to add this to spec?

There was a thread on www-style and I did add it to the spec, but it's currently commented out in the spec source. There's also https://www.w3.org/Bugs/Public/show_bug.cgi?id=17098

Comment 14

[Simon Pieters [:zcorpan]]

(In reply to Simon Pieters from comment #8)
> It's not case-insensitive as far as I can tell.
> http://software.hixie.ch/utilities/js/live-dom-viewer/saved/2732

...however, webkitFoo and WebkitFoo both work, and it currently works to mix dashes and uppercase e.g. style['border-rightColor'].

Comment 15

[Boris Zbarsky [:bzbarsky]]

What about in IE?

If we do standardize this, I'd like to standardize the least-insane subset possible, obviously, and 'border-rightColor' is over the boundary.  ;)

Comment 16

[Simon Pieters [:zcorpan]]

IE11 is case-sensitive for camelCase but case-insensitive for dashed properties. Mixing dashes and not dashes is not supported.

In IE, fontSize lives on the prototype but font-size is on the object. In Blink both are on the object (but that might change as part of https://code.google.com/p/chromium/issues/detail?id=43394 - not sure).

The current spec says fontSize should be on the prototype. The commented out spec for dashed properties says they should also be on the prototype, only lowercase is supported and no mixing of dashes and uppercase.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Sounds good.  That's the intersection of what Trident and Blink do, so should be reasonable.

Comment 19

[Hallvord R. M. Steen [:hallvors]]

Seems Chrome's usage stats confirm that this is a common problem: https://code.google.com/p/chromium/issues/detail?id=290055#c9 (though some people question whether it is because pages set both hyphenated and camel case - I can't say I've personally noticed scripts doing that..)

Regarding border-rightColor and friends, I suppose what's happening there is some "fallback" algorithm that does a hyphen-to-camel-case conversion if unknown property names with hyphens in are used. While this looks messy from an author point of view, and would indeed be crazy to implement by adding all possible variations to the prototype, I think it would make sense to support this too if it's just adding a couple of lines to an algorithm.

Comment 20

[Simon Pieters [:zcorpan]]

(In reply to Hallvord R. M. Steen from comment #19)
> Regarding border-rightColor and friends, I suppose what's happening there is
> some "fallback" algorithm that does a hyphen-to-camel-case conversion if
> unknown property names with hyphens in are used. While this looks messy from
> an author point of view, and would indeed be crazy to implement by adding
> all possible variations to the prototype, I think it would make sense to
> support this too if it's just adding a couple of lines to an algorithm.

Mixed forms are no longer supported in Blink. https://code.google.com/p/chromium/issues/detail?id=334562

Comment 21

[Hallvord R. M. Steen [:hallvors]]

That's good news, thank you Simon :-)

Comment 22

[Hallvord R. M. Steen [:hallvors]]

(Note that this breaks Google Images touch UI, might be worth prioritising because we haven't been able to get them to fix it this far..)

Comment 23

[Boris Zbarsky [:bzbarsky]]

Assuming we want to make the change at all, of course.

Comment 24

[Simon Pieters [:zcorpan]]

FYI https://code.google.com/p/chromium/issues/detail?id=290055 is now WONTFIXed.

Comment 25

[Mike Taylor [:miketaylr]]

Thanks Simon.

> The consensus on  bug 290055  is that we can't drop the corresponding
functionality because of high usage both on the counters and in
libraries.

To me, this suggests we should seriously consider adding support for this (and getting a compat note somewhere in the relevant spec.)

Comment 26

[Simon Pieters [:zcorpan]]

https://www.w3.org/Bugs/Public/show_bug.cgi?id=17098 - the spec now includes dashed attributes again.
http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-dashed-attribute

Comment 27

[Brad Lassey [:blassey] (use needinfo?)]

flagging for triage, this would resolve some compatibility problems, such as bug 793216

Comment 28

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

Yeah, now that this is in the spec, we should do this.

bz/heycam -- how much work does this look like?  (I'm not sure whether the wording about what's on the object vs. prototype is hard to do in our webidl setup.)


No idea what it has to do with e10s, though.

Comment 29

[Brad Lassey [:blassey] (use needinfo?)]

Sorry, meant to flag for fennec, not e10s

Comment 30

[Boris Zbarsky [:bzbarsky]]

> bz/heycam -- how much work does this look like?

I was actually just looking into this.  The current spec wording basically treats these properties like any other IDL attribute on the prototype, as far as I can tell.

Implementing that should be pretty simple.  Patches coming up in a bit.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add support for doing BinaryName annotations in IDL

Comment 32

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Pass the actual names of the CSS properties involved to GenerateCSS2PropertiesWebIDL.py

Comment 33

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Fix codegen support for IDL attribute names and methods with dashes in them

Comment 34

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Generate property accessors on the CSS2Properties interface for the dashed-named CSS properties and float that use their actual property names

Comment 35

[Boris Zbarsky [:bzbarsky]]

One note: This implements distinct specialized getters/setters which call the same C++ implementation.

We could try to do something more complicated in codegen where we'd use the specialized getter/setter for the non-dashed version, though the error reporting would look weird.  I wasn't sure whether that complexity was really worth it, though admittedly it would avoid the need for most of part 3.

Comment 36

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/0933d8d6fc10
   https://hg.mozilla.org/integration/mozilla-inbound/rev/e3adea0bd276
   https://hg.mozilla.org/integration/mozilla-inbound/rev/aeaee069ed50
   https://hg.mozilla.org/integration/mozilla-inbound/rev/84e73f166d21

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0933d8d6fc10
https://hg.mozilla.org/mozilla-central/rev/e3adea0bd276
https://hg.mozilla.org/mozilla-central/rev/aeaee069ed50
https://hg.mozilla.org/mozilla-central/rev/84e73f166d21