Closed Bug 892929 Opened 6 years ago Closed 6 years ago

CSSFontFeatureValuesRule seems unaffected by the pref

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25
Tracking Status
firefox23 --- unaffected
firefox24 + fixed
firefox25 + fixed

People

(Reporter: Ms2ger, Assigned: jtd)

References

Details

Attachments

(2 files)

window.CSSFontFeatureValuesRule should be undefined when layout.css.font-features.enabled is false, but it isn't.
This is an xpidl interface, not a WebIDL one, so not defining it on Window is kinda hard...

We should just convert to WebIDL ASAP.  ;)
WebIDL doesn't seem backportable (which we might want), and on old bindings it should only take 5 lines or so in OldBindingConstructorEnabled, I think.
(In reply to :Ms2ger from comment #2)
> WebIDL doesn't seem backportable (which we might want), and on old bindings
> it should only take 5 lines or so in OldBindingConstructorEnabled, I think.

Is the behavior not working as expected as well or is it only a front end issue ? 

Also is this "window.CSSFontFeatureValuesRule" a new property given implementation of Bug 549861 ? Under what scenario is this pref generally used and how can a user be impacted ?
The pref is set to false in release builds: <http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/init/all.js#1778>. This property is new in bug 549861; exposing it when we turn off the actual implementation through the pref can break feature detection and probably breaks our experimental implementations policy.
There's a related bug to make tests not fail when the pref is disabled, bug 886691.  That's more about making the logic of shorthands work and fixing up tests that aren't sensitive to the pref but I thought I'd mention it here.
> Also is this "window.CSSFontFeatureValuesRule" a new property given
> implementation of Bug 549861 ? Under what scenario is this pref generally
> used and how can a user be impacted ?

Like other new CSS properties and rules behind prefs, this will generally be enabled in nightly/aurora builds but disabled in beta/release builds.  The pref will be removed once we make the decision that the spec is stable enough to unpref it.  I think that process is still a little undefined as we haven't done this yet for a lot of preffed-rather-than-prefixed properties.
Assignee: nobody → jdaggett
Bouncing this over to bz as I don't really know the right way to go about hiding window.CSSFontFeatureValuesRule based on the pref state.
Assignee: jdaggett → bzbarsky
Attached patch Per comment 2Splinter Review
This applies to today's Aurora as well.

John, can you deal with writing tests and shepherding this in?  I'm in and out a bit between now and the Aurora merge.  :(
Attachment #782754 - Flags: review?(bugs)
Flags: needinfo?(jdaggett)
Assignee: bzbarsky → jdaggett
Comment on attachment 782754 [details] [diff] [review]
Per comment 2

But this needs those tests.
Attachment #782754 - Flags: review?(bugs) → review+
Attachment #782956 - Flags: review?(bzbarsky)
Flags: needinfo?(jdaggett)
(In reply to Boris Zbarsky (:bz) from comment #8)
> John, can you deal with writing tests and shepherding this in?  I'm in and
> out a bit between now and the Aurora merge.  :(

Yup, no problem.
Comment on attachment 782956 [details] [diff] [review]
patch, test whether window.CSSFontFeatureRule is hidden when disabled

>+  var hasFFVRule = (window["CSSFontFeatureValuesRule"] != null);

  var hasFFVRule = "CSSFontFeatureValuesRule" in window;

and similar for hasStyleAlternates and hasCompStyleAlternates.

r=me with that
Attachment #782956 - Flags: review?(bzbarsky) → review+
Thanks!
https://hg.mozilla.org/mozilla-central/rev/60dcd30de3fb
https://hg.mozilla.org/mozilla-central/rev/9b235393dd59
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
The description in that doc doesn't appear accurate.  This change is substantially different from the one above it (about geolocation).  And this is also a new feature that's not turned on yet, so the effect of this bug should be to make it so that there is no change in Firefox 25.  I think it should be removed from the doc.
Flags: needinfo?(kohei.yoshino.bugs)
But this does need to be backported to 24.
Flags: needinfo?(jdaggett)
Thanks for the feedback, removed the reference from the compatibility doc.
Flags: needinfo?(kohei.yoshino.bugs)
Comment on attachment 782754 [details] [diff] [review]
Per comment 2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 549861
User impact if declined: exposure of CSSFontFeatureValuesRule on window
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none
Attachment #782754 - Flags: approval-mozilla-aurora?
Comment on attachment 782956 [details] [diff] [review]
patch, test whether window.CSSFontFeatureRule is hidden when disabled

[Approval Request Comment]
-- same as above --
Attachment #782956 - Flags: approval-mozilla-aurora?
Attachment #782754 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #782956 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Looks like this landed with tests. Any need for QA here?
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.