91370 - Implement setting cssText on various CSSRule implementations

Status: NEW

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

Description

[Boris Zbarsky [:bzbarsky]]

setting cssText doesn't work anywhere at the moment.

It needs to work in:

CSSCharsetRuleImpl
CSSImportRuleImpl
CSSMediaRuleImpl
CSSNameSpaceRuleImpl
CSSStyleRuleImpl
nsDOMCSSDeclaration

Most of the work for nsDOMCSSDeclaration is done already, so making that work is
a one-line change:

-  // XXX TBI
-  return NS_OK;
+  return ParseDeclaration(aCssText, PR_FALSE, PR_TRUE);

Arguably, this is the most likely-to-be-used case, so it may be worth checking
in on its own.

Comment 1

[Johnny Stenback (:jst)]

Peter, would you please review bz's patch in his comment above?

Comment 2

[Peter Van der Beken [:peterv]]

r=peterv.

Comment 3

[Johnny Stenback (:jst)]

sr=jst, bz, would you be able to check this one in?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Checked in.  Leaving open for now... we could make this the SetCssText metabug
or resolve it and file a bunch of new bugs..

Comment 5

[Johnny Stenback (:jst)]

Futuring.

Comment 6

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 7

[Anne (:annevk)]

Still needed? (Or: has anything been decided past 4 years?)

Comment 8

[Anne (:annevk)]

Ignore comment 7. It seems setting cssText is not implemented. At least not for cases like

  document.styleSheets[0].cssRules[0].cssText = "p { background:lime }"

although it doesn't throw either.

On setting it should also be checked if the statement conforms to the syntax rules. If not, SYNTAX_ERR must be thrown. If one of the declarations is invalid, it should be dropped and not form part of the setted value. And you also should not be allowed to change the type of rule. That raises a INVALID_MODIFICATION_ERR exception...

Comment 9

[Boris Zbarsky [:bzbarsky]]

> If not, SYNTAX_ERR must be thrown.

I don't think we want to do this.  Unknown CSS is Unknown CSS -- this case should not differ from the invalid declaration case; in fact the two cannot be told apart.

Comment 10

[Anne (:annevk)]

Currently you throw for `p:-test- { background:lime }` for example, but not for `p { foo:bar }` (using insertRule).

Comment 11

[Boris Zbarsky [:bzbarsky]]

Yeah, that's what the DOM2 CSS spec says; I now repent of implementing it that way.  I think we might even have a bug on it; I know dbaron has commented about it in the past.

Comment 12

[Benjamin Stover (:stechz)]

Is there a reason the patch in comment 1 has not landed?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Assuming you mean comment0, it has.  See comment 4.

Comment 14

[Benjamin Stover (:stechz)]

Yes, sorry, I meant #0.  I am confused because it doesn't seem to be landed:
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSStyleRule.cpp#1511

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment 0 pretty clearly talks about nsDOMCSSDeclaration, no?  http://hg.mozilla.org/mozilla-central/annotate/98c37ac79fdc/layout/style/nsDOMCSSDeclaration.cpp#l121

Comment 16

[Karl Dubost💡 :karlcow]

This came up in an issue about Google Drive.
https://webcompat.com/issues/3822

Comment 17

[Boris Zbarsky [:bzbarsky]]

That doesn't look relevant to this bug at all.  The code linked there is doing:

  e = fb(d.querySelectorAll("*"));
...
                   var g = e[f],  
...
                g.style.cssText = l;

so I'm pretty sure "g" is an element.  Setting .cssText on an element's .style works perfectly fine and has for 15 years, ever since comment 4.

The real issue you seem to be running into there is that _getting_ .cssText on computed style always returns an empty string in Gecko, whereas in Chrome it returns a very long string of all the computed styles.  Looks like bug 137687 tracks this; this bug does NOT block that one.

Comment 18

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 19

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.