Closed
Bug 677302
Opened 13 years ago
Closed 9 years ago
Support putting UA CSS style sheet rules behind a boolean preferences
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
DUPLICATE
of bug 1267890
People
(Reporter: jwalker, Unassigned)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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?
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. :)
Comment 2•13 years ago
|
||
We can pretty easily allow the syntax in chrome sheets only, fwiw.
Updated•13 years ago
|
Version: unspecified → Trunk
Updated•13 years ago
|
Component: DOM: Mozilla Extensions → Style System (CSS)
QA Contact: general → style-system
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...
Comment 4•13 years ago
|
||
This doesn't look like it handles dynamic changes, but otherwise seems reasonable.
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'.
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.
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)
Updated•13 years ago
|
Keywords: sec-review-needed
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.
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.
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");
}
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.
Updated•13 years ago
|
Updated•13 years ago
|
Keywords: privacy-review-needed
sec review triage = need to sched a full review
Updated•13 years ago
|
Whiteboard: [secr:curtisk]
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.
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.
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.
Comment 16•13 years ago
|
||
The patch is a WIP :)
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).
Updated•13 years ago
|
Keywords: privacy-review-needed
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).
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.
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.
Updated•13 years ago
|
Blocks: writing-mode
Updated•13 years ago
|
Whiteboard: [secr:curtisk] → [sec-assigned:curtisk:749363]
Updated•12 years ago
|
Flags: sec-review?(curtisk)
Updated•12 years ago
|
Whiteboard: [sec-assigned:curtisk:749363]
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.
Summary: Preffing out CSS should be easier → Support putting UA CSS style sheet rules behind a boolean preferences
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
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.
Attachment #552826 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #552256 -
Attachment description: wip → wip, @-moz-preference
Comment 25•11 years ago
|
||
Mats, are you planning to finish this, or should jwatt or somebody else take it over?
Flags: needinfo?(matspal)
Comment 26•11 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?
Flags: needinfo?(dbaron)
Comment 27•11 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.
Comment 28•11 years ago
|
||
Bug 935803 comment 37 is another reason to want a fix here.
dbaron? Any thoughts on comment 26?
Comment 29•10 years ago
|
||
jwatt, feel free to take this; I'm probably not going to get to it anytime soon.
Flags: needinfo?(mats)
Updated•10 years ago
|
Flags: sec-review?(curtisk) → sec-review?
Comment 30•9 years ago
|
||
Yeah, comment 23 is reasonable. But this got implemented in @supports in a separate bug, without reference to this one.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(dbaron)
Resolution: --- → DUPLICATE
Comment 31•9 years ago
|
||
Fantastic news! Thanks!
Updated•9 years ago
|
Updated•8 years ago
|
Comment 33•8 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.
Flags: needinfo?(mats)
Flags: needinfo?(jwatt)
Comment 34•8 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?)
Flags: needinfo?(jwatt)
Updated•8 years ago
|
Flags: needinfo?(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.
Flags: needinfo?(mconley)
Updated•2 years ago
|
Flags: needinfo?(MatsPalmgren_bugz)
You need to log in
before you can comment on or make changes to this bug.
Description
•