In bug 807880, CtP tries to avoid inheriting page styles by specifying "initial" for all the properties they could think of. This isn't robust or future-proof.

http://dev.w3.org/csswg/css3-cascade/#all-shorthand would fix this

This has also been an issue for video controls. It would be most handy to have this capability, I've seen the 'all' spec before and it sounds perfect.

https://tbpl.mozilla.org/?tree=Try&rev=1835823802cb (includes bug 921731 patches too)

Comment on attachment 811865 [details] [diff] [review] Part 0: Add "layout.css.all-shorthand.enabled" pref. r=me

Comment on attachment 811866 [details] [diff] [review] Part 1: Add "all" shorthand property. Maybe call this macro CSS_PROP_LIST_ONLY_COMPONENTS_OF_ALL_SHORTHAND ? >+#endif I'd appreciate a "// !defined(CSS_PROP_LIST_ONLY_COMPONENTS_OF_ALL_SHORTHAND)" after this. Why are some internal props excluded and some not? r=me with the above addressed

Comment on attachment 811867 [details] [diff] [review] Part 2: Parse the "all" shorthand property. r=me

Comment on attachment 811868 [details] [diff] [review] Part 3: Serialize the "all" shorthand property as the empty string unless all components are inherit/initial/unset. The code and commit message don't agree... Seems to me like this _always_ serializes "all" as the empty string, no? r=me with (presumably) the comment comment fixed.

Comment on attachment 811869 [details] [diff] [review] Part 4: Test. > +function set(array) { Why not use actual JS Set() here? >+ var initialComputedValues = { }; >+ var otherComputedValues = { }; And Map() here? >+ // Test setting all:inherit through setProperty. ... >+ parentRule.style.setProperty(prop, info.other_values[0], ""); >+ childRule2.style.setProperty("all", "inherit"); I think you should childRule1.style.setProperty(prop, "initial") here. Otherwise you could pass the test for inherited properties even if "all: inherit" did absolutely nothing. >+ // Test setting all:initial through setProperty. ... >+ parentRule.style.setProperty(prop, info.other_values[0], ""); >+ childRule2.style.setProperty("all", "initial"); And childRule1.style.setProperty(prop, "inherit") here, or info.other_values[0] instead of "inherit", for the same reason but for reset properties. r=me with those last two comments fixed. Either way on Map and Set, but seems cleaner to use those, in general.

(In reply to Boris Zbarsky [:bz] from comment #11) > Why are some internal props excluded and some not? I found that doing button { all: unset; } would still result in some borders being painted if I excluded the internal border properties, not exactly sure why though. (In reply to Boris Zbarsky [:bz] from comment #13) > The code and commit message don't agree... Seems to me like this _always_ > serializes "all" as the empty string, no? I should just update the comment. Earlier in the function we serialise to "inherit", "initial" or "unset", if all of the longhand components are one of those values. In all other cases we serialise to "".

I notice that CSS Cascading and Inheritance Level 3, where this (and "unset" from bug 921731) are defined, is in Candidate Recommendation. Does that means I don't need to hide this behind an off-for-release-builds pref?

> I found that doing So I think the key is that some props are internal in the sense that pages can't set them directly, but are actually set by exposed shorthand props, right? The definition of the macro should probably document this, so people adding new props know when to use it or not. > Does that means I don't need to hide this behind an off-for-release-builds pref? That's a question for dbaron; I haven't been following the WG's work that closely.

(In reply to Boris Zbarsky [:bz] from comment #17) > > Does that means I don't need to hide this behind an off-for-release-builds pref? > > That's a question for dbaron; I haven't been following the WG's work that > closely. Yeah, I think we're fine to ship this. That's generally the case for CRs, though there might be a few ancient ones where that's not the case.

OK. I'll keep the pref for the moment but not conditionalise it on RELEASE_BUILD in all.js.

(In reply to Boris Zbarsky [:bz] from comment #17) > So I think the key is that some props are internal in the sense that pages > can't set them directly, but are actually set by exposed shorthand props, > right? The definition of the macro should probably document this, so people > adding new props know when to use it or not. That makes sense. The UA style sheet sets the internal property via the shorthand, and unset:all only targets longhands.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a08adaae0322 https://hg.mozilla.org/integration/mozilla-inbound/rev/998003ad5546 https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5c63580196 https://hg.mozilla.org/integration/mozilla-inbound/rev/3139f05cbe1d https://hg.mozilla.org/integration/mozilla-inbound/rev/60c43e6a1489

https://hg.mozilla.org/mozilla-central/rev/60c43e6a1489 https://hg.mozilla.org/mozilla-central/rev/3139f05cbe1d https://hg.mozilla.org/mozilla-central/rev/ff5c63580196 https://hg.mozilla.org/mozilla-central/rev/998003ad5546 https://hg.mozilla.org/mozilla-central/rev/a08adaae0322

(In reply to Boris Zbarsky [:bz] (Vacation Oct 12 - Oct 27) from comment #11) > Comment on attachment 811866 [details] [diff] [review] > Part 1: Add "all" shorthand property. > > Maybe call this macro CSS_PROP_LIST_ONLY_COMPONENTS_OF_ALL_SHORTHAND ? > Why are some internal props excluded and some not? (In reply to Boris Zbarsky [:bz] (Vacation Oct 12 - Oct 27) from comment #17) > So I think the key is that some props are internal in the sense that pages > can't set them directly, but are actually set by exposed shorthand props, > right? The definition of the macro should probably document this, so people > adding new props know when to use it or not. It seems like the key distinction is that: * internal properties that are set indirectly by some other shorthand (but used to internally divide it up) should be part of 'all' * internal properties that represent things that aren't supposed to be part of CSS, but that we implement as part of CSS, should not be part of 'all' By this logic, I see one mistake: -x-system-font should be part of the 'all' shorthand. It represents the part of the 'font' shorthand that is not represented by any of the subproperties. heycam, any chance you could: * extend the comment to reflect this distinction, and * fix -x-system-font

(In reply to David Baron [:dbaron] (needinfo? me) from comment #23) > heycam, any chance you could: > * extend the comment to reflect this distinction, and > * fix -x-system-font Yes, filed bug 925218 for it.

I've looked through the test runs on tbpl containing the tests from the patch of this bug, and they all look ok. Are there any other tests not included in this patch, that I should be looking for? Thanks!

No, that's all the tests for 'all' so far.

Marking this as verified, based on comment 25 and comment 26.

Doc updated: https://developer.mozilla.org/en-US/Firefox/Releases/27 https://developer.mozilla.org/en-US/docs/Web/CSS/all