style: Make Servo deal with CSS property prefs more correctly.

Right now you could still set preffed-off properties from CSSStyleDeclaration.

4 files changed, 54 insertions, 16 deletions

diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
index 2699bda9b07..37b1a0a4ae1 100644
--- a/components/script/dom/cssstyledeclaration.rs
+++ b/components/script/dom/cssstyledeclaration.rs
@@ -164,9 +164,17 @@ macro_rules! css_properties(
     ( $([$getter:ident, $setter:ident, $id:expr],)* ) => (
         $(
             fn $getter(&self) -> DOMString {
+                debug_assert!(
+                    $id.enabled_for_all_content(),
+                    "Someone forgot a #[Pref] annotation"
+                );
                 self.get_property_value($id)
             }
             fn $setter(&self, value: DOMString) -> ErrorResult {
+                debug_assert!(
+                    $id.enabled_for_all_content(),
+                    "Someone forgot a #[Pref] annotation"
+                );
                 self.set_property($id, value, DOMString::new())
             }
         )*
@@ -238,6 +246,10 @@ impl CSSStyleDeclaration {
             return Err(Error::NoModificationAllowed);
         }
 
+        if !id.enabled_for_all_content() {
+            return Ok(());
+        }
+
         self.owner.mutate_associated_block(|pdb, changed| {
             if value.is_empty() {
                 // Step 3
diff --git a/components/script/dom/webidls/CSSStyleDeclaration.webidl b/components/script/dom/webidls/CSSStyleDeclaration.webidl
index b6df0176947..1bb20990494 100644
--- a/components/script/dom/webidls/CSSStyleDeclaration.webidl
+++ b/components/script/dom/webidls/CSSStyleDeclaration.webidl
@@ -244,8 +244,10 @@ partial interface CSSStyleDeclaration {
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString whiteSpace;
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString white-space;
 
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString writingMode;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString writing-mode;
+  [Pref="layout.writing-mode.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString writingMode;
+  [Pref="layout.writing-mode.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString writing-mode;
 
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString letterSpacing;
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString letter-spacing;
@@ -385,13 +387,20 @@ partial interface CSSStyleDeclaration {
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString imageRendering;
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString image-rendering;
 
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString columnCount;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString column-count;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString columnWidth;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString column-width;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString columns;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString columnGap;
-  [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString column-gap;
+  [Pref="layout.column-count.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString columnCount;
+  [Pref="layout.column-count.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString column-count;
+  [Pref="layout.column-width.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString columnWidth;
+  [Pref="layout.column-width.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString column-width;
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString columns;
+  [Pref="layout.column-gap.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString columnGap;
+  [Pref="layout.column-gap.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  attribute DOMString column-gap;
 
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString transition;
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString transitionDuration;
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 19541d6d79d..a3a33ee97bd 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -1626,16 +1626,35 @@ impl PropertyId {
         }
     }
 
-    fn allowed_in(&self, context: &ParserContext) -> bool {
-        let id: NonCustomPropertyId = match *self {
-            // Custom properties are allowed everywhere
-            PropertyId::Custom(_) => return true,
+    fn non_custom_id(&self) -> Option<NonCustomPropertyId> {
+        Some(match *self {
+            PropertyId::Custom(_) => return None,
             PropertyId::Shorthand(shorthand_id) => shorthand_id.into(),
             PropertyId::Longhand(longhand_id) => longhand_id.into(),
             PropertyId::ShorthandAlias(_, alias_id) => alias_id.into(),
             PropertyId::LonghandAlias(_, alias_id) => alias_id.into(),
+        })
+    }
+
+    /// Whether the property is enabled for all content regardless of the
+    /// stylesheet it was declared on (that is, in practice only checks prefs).
+    #[inline]
+    pub fn enabled_for_all_content(&self) -> bool {
+        let id = match self.non_custom_id() {
+            // Custom properties are allowed everywhere
+            None => return true,
+            Some(id) => id,
         };
 
+        id.enabled_for_all_content()
+    }
+
+    fn allowed_in(&self, context: &ParserContext) -> bool {
+        let id = match self.non_custom_id() {
+            // Custom properties are allowed everywhere
+            None => return true,
+            Some(id) => id,
+        };
         id.allowed_in(context)
     }
 }
@@ -3818,8 +3837,7 @@ impl fmt::Debug for AliasId {
     }
 }
 
-// FIXME(emilio): This macro doesn't account for experimental properties, so
-// even with the pref disabled you can set them from CSSOM in Servo.
+// NOTE(emilio): Callers are responsible to deal with prefs.
 #[macro_export]
 macro_rules! css_properties_accessors {
     ($macro_name: ident) => {
diff --git a/resources/prefs.json b/resources/prefs.json
index 0c42ab221ec..8c7507f73f2 100644
--- a/resources/prefs.json
+++ b/resources/prefs.json
@@ -61,7 +61,6 @@
   "layout.column-gap.enabled": false,
   "layout.column-width.enabled": false,
   "layout.columns.enabled": false,
-  "layout.text-orientation.enabled": false,
   "layout.viewport.enabled": false,
   "layout.writing-mode.enabled": false,
   "network.http-cache.disabled": false,