Joe Walker [:jwalker] Reporter
	Description • 13 years ago

Example use:

@-moz-pref(slimline.feature.enabled) {
  .blah: margin-left: 2px;
}

.blah: margin-left: 6px;

Alternatives:
Currently it seems that the best option is to have JS reflect pref state in a parent node, and have 'preffed out CSS' include the reflected pref state in some way.

Pros:
The option above (along the lines of @-moz-document) could make it cleaner to develop features that are preffed out.

Cons:
Is this too much of a point solution - one that only applies to a limitted set of cases?

	Justin Dolske [:Dolske]
	Comment 1 • 13 years ago

I like the idea, it would be useful in front-end code.

Perhaps obviously, it would need to be chrome-only to prevent sniffing/fingerprinting user prefs by untrusted content. Or the "pref" namespace could just be separate from the UA prefs... document.setStylePref() or Cc[xxx].getService(Ci.nsICSSFoo).setStylePref(). Details. :)

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 13 years ago

We can pretty easily allow the syntax in chrome sheets only, fwiw.

	Mats Palmgren (inactive)
	Comment 3 • 13 years ago

This implements the following syntax:
  '@-moz-preference' '(' <pref-name> ',' <pref-value> ')' '{' <rule>* '}'
where
   pref-name: <string>
   pref-value: [ false | true | <string> | <integer> ]

when pref-value is false/true use Preferences::GetBool to lookup the value,
<string> use Preferences::GetString and <integer> use Preferences::GetInt.

I made @-moz-preference("pref", false) special in the sense that
it matches also when "pref" does not exist, is this what we want?

Currently there are no restrictions where @-moz-preference can be used...

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 4 • 13 years ago

This doesn't look like it handles dynamic changes, but otherwise seems reasonable.

	Joe Walker [:jwalker] Reporter
	Comment 5 • 13 years ago

(In reply to Mats Palmgren [:mats] from comment #3)
> I made @-moz-preference("pref", false) special in the sense that
> it matches also when "pref" does not exist, is this what we want?

I can't properly answer for 'we', but I can generalize from me: Yes.

The primary use for me is in enabling features which are preffed off, which frequently means that the pref does not exist.


I can see there are times when non-dynamic behaviour is what is wanted. It could be reasonable to be non-dynamic in JS about a pref to turn on a feature that is intended to be on permanently.

However in this case it's perhaps reasonable to have the instructions 'add this pref and then immediately restart if you don't want things to look wierd'.

	Johnathan Nightingale [:johnath]
	Comment 6 • 13 years ago

This is hot like a fire.

(In reply to Mats Palmgren [:mats] from comment #3)
> Currently there are no restrictions where @-moz-preference can be used...

If that's so, then I think, dolske's point in comment 1 bears repeating: we should only match against prefs in an otherwise-empty branch (devtools.css.*? devtools.mozpref.*? css.mozpref.*?) so that it can't be used for content-space pref-sniffing.

Also this is a hot like a fire.

	Johnathan Nightingale [:johnath]
	Comment 7 • 13 years ago

(Adding Curtis and Jesse as cc's to watch for the gotchas because I love this idea very much and don't want it to bite us)

	Mats Palmgren (inactive)
	Comment 8 • 13 years ago

(In reply to David Baron [:dbaron] from comment #4)
> This doesn't look like it handles dynamic changes, ...

Right, dynamic changes are hard to do effeciently with the
@-moz-preference rule approach.  Fortunately all that hard work has
already been done for @media queries!  So I changed it to a @media query
expression instead.  Then we get effecient dynamic updates for free,
plus you can write more powerful expressions.

This patch implements a -moz-preference expression syntax like so:
'(' '-moz-preference' <pref-name> ':' <pref-value> ')'
  where pref-name: <string>
        pref-value: [ false | true | <string> | <integer> ]

and you can put that in a @media rule anywhere an expression is allowed
(http://www.w3.org/TR/css3-mediaqueries/), for example:

@media print and (-moz-preference "print.print_color" : true) {
  ...
}

In this patch I've restricted -moz-preference to agent/user/override
sheets.  In other sheets a -moz-preference expressions does not match
regardless of the pref value.  It's still parsed though, so you can have
@media (-moz-preference "print.print_color" : true) , screen {
  ...
}
in a document sheet and it would match for screen media.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 9 • 13 years ago

Dynamic changes shouldn't be any easier or harder to handle either way; I think we should decide on the syntax based on which makes more sense.

	Jesse Ruderman
	Comment 10 • 13 years ago

Would it make sense for -moz-preference to work as a value in addition to a selector? Then we could fix bug 648218 without insane JS.

tabbrowser-tab:not([pinned]) { 
  max-width: -moz-preference("browser.tabs.tabMaxWidth");
}

	Dão Gottwald [:dao]
	Comment 11 • 13 years ago

(In reply to Jesse Ruderman from comment #10)
> Would it make sense for -moz-preference to work as a value in addition to a
> selector?

This sounds like a separate feature.

	Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]
	Comment 12 • 13 years ago

sec review triage = need to sched a full review

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 13 • 13 years ago

Curtis: As far as I know the plan is to limit this to chrome stylesheets only, per comment 2, so I don't see why this would need privacy/security review.

	Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]
	Comment 14 • 13 years ago

Gavin: I don't pretend to understand why the team makes some of the decisions they do, but multiple members wanted to do a review. They generally do have concerns over things that touch chrome. If it is really simple and they see no immediate interest the review will be very short.
That said, if you could pick a date (https://mail.mozilla.com/home/ckoenig@mozilla.com/Security Review.html) that works for the needed knowledgeable people that would be appreciated.

	Jesse Ruderman
	Comment 15 • 13 years ago

I looked at the parts of the patch that mention AllowMozPreference. Looks like the parsing code isn't constrained to agent sheets. Why not, and how much attack surface does that add?

What is nsStyleSet::eOverrideSheet?

What are the implications of handling (or not handling) dynamic changes to prefs? Could a pref change occur at a time when it is not safe to do layout?

Will this feature make it more difficult to parallelize layout? IIRC prefs are main-thread only, and they're special in e10s (https://developer.mozilla.org/en/Multi-Process_Architecture/Multi_Process_Preference_System ?).

I'd like this patch to have tests, including tests to ensure web page content can't use the feature.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 16 • 13 years ago

The patch is a WIP :)

	Sid Stamm [:geekboy or :sstamm]
	Comment 17 • 13 years ago

This is awesome.  If it's restricted to chrome code (comment 2), there's no privacy risk at all.  Exposing this to content sounds kind of freaky unless we restrict it to a known empty subtree that is clearly for public consumption (comment 6, to avoid sniffing things like browser.privatebrowsing.autostart, services.sync.*, add-on prefs, etc).

	Sid Stamm [:geekboy or :sstamm]
	Comment 18 • 13 years ago

Dao: why did you remove the privacy-review-needed keyword?  Are the plans to make this only available to chrome-level code?  I had been waiting to clear the keyword since this feature is still WIP and may change (comment 16).

	Dão Gottwald [:dao]
	Comment 19 • 13 years ago

(In reply to Sid Stamm [:geekboy] from comment #18)
> Dao: why did you remove the privacy-review-needed keyword?  Are the plans to
> make this only available to chrome-level code?

Yes, that's the one and only plan.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 20 • 13 years ago

mats, bz, and I just had a quick discussion about this.

We want to do @-moz-preference rather than a media query, but it may be helpful to reuse nsMediaQueryResultCacheKey (etc.) mechanisms for helping with the dynamic change handling.

We definitely want to allow this in UA sheets and in chrome sheets.  User sheets is an open question, but perhaps leaning towards allowing.

	Jonathan Watt [:jwatt]
	Comment 22 • 11 years ago

(In reply to David Baron [:dbaron] (needinfo? me) (UTC+8) from comment #20)
> We definitely want to allow this in UA sheets and in chrome sheets.  User
> sheets is an open question, but perhaps leaning towards allowing.

Let's make this only about UA style sheets for now, and deal with exposing the mechanism to user style sheets (and the associated security review) as a follow-up bug. We shouldn't delay making this available to UA sheets because we're not sure about the latter.

	Mats Palmgren (inactive)
	Comment 23 • 11 years ago

I tend to think an MQ expression is the better option, because you can combine
it with existing expressions for a richer query language.  That also allows
prefs to be used in the 'media' attribute of <link> / <style>, and in @import
etc for free.  MQ gives nice synergy effects that @-moz-preference won't have.

FYI, "IndieUI User Context" propose a new Media Query feature that is similar:
https://dvcs.w3.org/hg/IndieUI/raw-file/default/src/indie-ui-context.html

There was a brief discussion on www-style about it (Review of IndieUI User Context):
http://lists.w3.org/Archives/Public/www-style/2013Nov/thread.html#msg182

	Mats Palmgren (inactive)
	Comment 24 • 11 years ago

Here's an up-to-date trunk patch for the media query solution (as described
in comment 8), in case anyone wants play around with it.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 25 • 11 years ago

Mats, are you planning to finish this, or should jwatt or somebody else take it over?

	Jonathan Watt [:jwatt]
	Comment 26 • 10 years ago

David, as I understand it this stalled because your comment 20 is at odds with Mats's revised opinion expressed in comment 23. Can you review and come to some agreement with Mats so that this can move forward?

	James Craig
	Comment 27 • 10 years ago

(In reply to Mats Palmgren (:mats) from comment #23)
> I tend to think an MQ expression is the better option, because you can
> combine
> it with existing expressions for a richer query language.  That also allows
> prefs to be used in the 'media' attribute of <link> / <style>, and in @import
> etc for free.  MQ gives nice synergy effects that @-moz-preference won't
> have.
> 
> FYI, "IndieUI User Context" propose a new Media Query feature that is
> similar:
> https://dvcs.w3.org/hg/IndieUI/raw-file/default/src/indie-ui-context.html
> 
> There was a brief discussion on www-style about it (Review of IndieUI User
> Context):
> http://lists.w3.org/Archives/Public/www-style/2013Nov/thread.html#msg182

We're getting close to a FPWD, but significantly reduced the reliance on media queries. 
https://dvcs.w3.org/hg/IndieUI/raw-file/default/src/indie-ui-context.html

The primary interface is through an extension to the Window object that would allow you to request a setting such as:

  window.userSetting('user-font-size');

Vendor-specific preferences are also allowed, either for proposed standardization or for the scenario you're discussing above:

  window.userSetting('-moz-browser-tabs-tabMaxWidth');


There are still media features defined for some of the keys. Here's an example from the spec using 'user-font-size' to determine if layout should be single- or multi-column.

  /* Default layout uses 2 columns */
  main {
    columns: 2;
  }

  /* But if the user's default font size (from browser text zoom setting or... */
  /* user style sheet...) is larger than 32px, drop the columns. */
  /* Note: the CSS3 syntax is (min-user-font-size: 32) */
  /* Note: this example uses the greater-than/less-than syntax likely to be adopted by CSS4. */
  @media (user-font-size > 32) { 
    main {
      columns: auto;
    }
  }

Please provide feedback to public-indie-ui@w3.org. Thanks.

	Jonathan Watt [:jwatt]
	Comment 28 • 10 years ago

Bug 935803 comment 37 is another reason to want a fix here.

dbaron? Any thoughts on comment 26?

	Mats Palmgren (inactive)
	Comment 29 • 10 years ago

jwatt, feel free to take this; I'm probably not going to get to it anytime soon.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 30 • 8 years ago

Yeah, comment 23 is reasonable.  But this got implemented in @supports in a separate bug, without reference to this one.

	Justin Dolske [:Dolske]
	Comment 31 • 8 years ago

Fantastic news! Thanks!

	Mike Conley (:mconley) (:⚙️)
	Comment 33 • 7 years ago

Hey mats - how feasible do you think it'd be to dust off attachment 8337489 [details] [diff] [review] and rebase it on tip? The ability to pref on certain styles like this would be extremely helpful for the Photon project (especially as the theme-ing and animation teams would prefer to land things now rather than all at once when 57 hits central[1]).

Or jwatt, do you have the cycles for this one, along with the SVG work you're doing?

[1]: Yes, a build-time pref could also do the trick here, but having run-time pref support would help with the efforts going on in bug 1352069 as well.

	Jonathan Watt [:jwatt]
	Comment 34 • 7 years ago

(In reply to Mike Conley (:mconley) from comment #33)
> Or jwatt, do you have the cycles for this one, along with the SVG work
> you're doing?

Not right now, unfortunately. :(

(Also it seems bug 1352069 is fixed now. Is this still important for specific things you guys are working on?)

	Mike Conley (:mconley) (:⚙️)
	Comment 35 • 7 years ago

(In reply to Jonathan Watt [:jwatt][back 18th May] from comment #34)
> 
> (Also it seems bug 1352069 is fixed now. Is this still important for
> specific things you guys are working on?)

I think it'd make it easier to do some of the larger scale theme-ing work that's still in the pipe for Photon. Right now, we're doing #ifdef's, meaning that it's kinda difficult to test both the "Photon" configuration and the "Default" configuration without doing a rebuild.