Bug 1890842 - Generate CSSPageDescriptors.webidl and change CSSPageRule to use CSSPageDescriptors for Style(). r=webidl,emilio

Refactor GenerateCSS2PropertiesWebIDL.py to generate both CSS2Properties and
also CSSPageDescriptors.

I had to adjust the margin property in Servo to ensure
`margin_{inline,block}_{start,end}` did not end up in CSSPageDescriptors.

I also removed the main function from the script since the arguments are
already inaccurately named as leftovers from its previous usage, and this
script is only invoked by function name by the build anyway.

Differential Revision: https://phabricator.services.mozilla.com/D209061

dom/bindings/Bindings.conf

@@ -146,6 +146,11 @@ DOMInterfaces = {
     'nativeType': 'nsDOMCSSDeclaration'
 },
 
+'CSSPageDescriptors': {
+    'nativeType': 'mozilla::dom::CSSPageRuleDeclaration',
+    'headerFile': 'mozilla/dom/CSSPageRule.h',
+},
+
 'CSSConditionRule': {
     'nativeType': 'mozilla::css::ConditionRule',
     'headerFile': 'mozilla/css/GroupRule.h',
@@ -2019,4 +2024,14 @@ TemplatedAttributes = {
     },
 ],
 
+'CSSPageDescriptors': [
+    {
+        'template': 'CSSPageDescriptor',
+        'getter': 'GetPropertyValue',
+        'setter': 'SetPropertyValue',
+        'argument': ('nsCSSPropertyID', 'id'),
+        'attrName': 'nsCSSProps::PropertyIDLName(id)',
+    },
+],
+
 }

dom/bindings/GenerateCSSPropListWebIDL.py

@@ -2,15 +2,15 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
-import argparse
+# This script generates properties/descriptor binding webidl based on
+# ServoCSSPropList.
+
 import runpy
-import sys
+
 
 # Generates a line of WebIDL with the given spelling of the property name
 # (whether camelCase, _underscorePrefixed, etc.) and the given array of
 # extended attributes.
-
-
 def generateLine(propName, extendedAttrs):
     return "  [%s] attribute [LegacyNullToEmptyString] UTF8String %s;\n" % (
         ", ".join(extendedAttrs),
@@ -18,7 +18,7 @@ def generateLine(propName, extendedAttrs):
     )
 
 
-def generate(output, dataFile):
+def generate(output, dataFile, ruleType, interfaceName, bindingTemplate, extraDecls=""):
     propsData = runpy.run_path(dataFile)["data"]
     output.write(
         """/* THIS IS AN AUTOGENERATED FILE.  DO NOT EDIT */
@@ -30,13 +30,15 @@ def generate(output, dataFile):
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 [Exposed=Window]
-interface CSS2Properties : CSSStyleDeclaration {
-"""
+interface """
+        + interfaceName
+        + " : CSSStyleDeclaration {\n"
+        + extraDecls
     )
 
     for p in propsData.values():
         # Skip properties which aren't valid in style rules.
-        if "Style" not in p.rules:
+        if ruleType not in p.rules:
             continue
 
         if p.type() == "alias":
@@ -48,7 +50,7 @@ interface CSS2Properties : CSSStyleDeclaration {
         else:
             propId = p.id
         extendedAttrs = [
-            "BindingTemplate=(CSS2Property, eCSSProperty_%s)" % propId,
+            "BindingTemplate=(%s, eCSSProperty_%s)" % (bindingTemplate, propId),
             "CEReactions",
             "SetterThrows",
             "SetterNeedsSubjectPrincipal=NonSystem",
@@ -116,14 +118,22 @@ interface CSS2Properties : CSSStyleDeclaration {
     output.write("};")
 
 
-def main():
-    parser = argparse.ArgumentParser()
-    parser.add_argument(
-        "preprocessorHeader", help="Header file to pass through the preprocessor"
+def generateCSS2Properties(output, dataFile):
+    generate(output, dataFile, "Style", "CSS2Properties", "CSS2Property")
+
+
+def generateCSSPageDescriptors(output, dataFile):
+    generate(
+        output,
+        dataFile,
+        "Page",
+        "CSSPageDescriptors",
+        "CSSPageDescriptor",
+        """ \
+  // TODO alaskanemily: bug 1894969.
+  // This is here because otherwise PutForwards doesn't work on
+  // CSSPageRule.style.
+  [CEReactions, SetterNeedsSubjectPrincipal=NonSystem, SetterThrows]
+  attribute UTF8String cssText;
+""",
     )
-    args = parser.parse_args()
-    generate(sys.stdout, args.preprocessorHeader)
-
-
-if __name__ == "__main__":
-    main()

dom/bindings/moz.build

@@ -195,7 +195,13 @@ if CONFIG["CC_TYPE"] == "gcc":
 if CONFIG["COMPILE_ENVIRONMENT"]:
     GeneratedFile(
         "CSS2Properties.webidl",
-        script="GenerateCSS2PropertiesWebIDL.py",
-        entry_point="generate",
+        script="GenerateCSSPropListWebIDL.py",
+        entry_point="generateCSS2Properties",
+        inputs=["!/layout/style/ServoCSSPropList.py"],
+    )
+    GeneratedFile(
+        "CSSPageDescriptors.webidl",
+        script="GenerateCSSPropListWebIDL.py",
+        entry_point="generateCSSPageDescriptors",
         inputs=["!/layout/style/ServoCSSPropList.py"],
     )

dom/tests/mochitest/general/test_interfaces.js

@@ -320,6 +320,8 @@ let interfaceNamesInGlobalScope = [
   // IMPORTANT: Do not change this list without review from a DOM peer!
   { name: "CSSNamespaceRule", insecureContext: true },
   // IMPORTANT: Do not change this list without review from a DOM peer!
+  { name: "CSSPageDescriptors", insecureContext: true },
+  // IMPORTANT: Do not change this list without review from a DOM peer!
   { name: "CSSPageRule", insecureContext: true },
   // IMPORTANT: Do not change this list without review from a DOM peer!
   { name: "CSSPropertyRule", insecureContext: true },

dom/webidl/CSSPageRule.webidl

@@ -11,5 +11,5 @@
 [Exposed=Window]
 interface CSSPageRule : CSSGroupingRule {
   attribute UTF8String selectorText;
-  [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
+  [SameObject, PutForwards=cssText] readonly attribute CSSPageDescriptors style;
 };

dom/webidl/moz.build

@@ -393,6 +393,7 @@ with Files("WebTransport*"):
 
 GENERATED_WEBIDL_FILES = [
     "CSS2Properties.webidl",
+    "CSSPageDescriptors.webidl",
 ]
 
 PREPROCESSED_WEBIDL_FILES = [

layout/style/CSSPageRule.cpp

@@ -6,6 +6,7 @@
 
 #include "mozilla/dom/CSSPageRule.h"
 #include "mozilla/dom/CSSPageRuleBinding.h"
+#include "mozilla/dom/CSSPageDescriptorsBinding.h"
 
 #include "mozilla/DeclarationBlock.h"
 #include "mozilla/ServoBindings.h"
@@ -50,6 +51,11 @@ nsISupports* CSSPageRuleDeclaration::GetParentObject() const {
   return Rule()->GetParentObject();
 }
 
+JSObject* CSSPageRuleDeclaration::WrapObject(
+    JSContext* aCx, JS::Handle<JSObject*> aGivenProto) {
+  return CSSPageDescriptors_Binding::Wrap(aCx, this, aGivenProto);
+}
+
 DeclarationBlock* CSSPageRuleDeclaration::GetOrCreateCSSDeclaration(
     Operation aOperation, DeclarationBlock** aCreated) {
   if (aOperation != Operation::Read) {
@@ -192,8 +198,6 @@ already_AddRefed<StyleLockedCssRules> CSSPageRule::GetOrCreateRawRules() {
   return Servo_PageRule_GetRules(mRawRule).Consume();
 }
 
-nsICSSDeclaration* CSSPageRule::Style() { return &mDecls; }
-
 JSObject* CSSPageRule::WrapObject(JSContext* aCx,
                                   JS::Handle<JSObject*> aGivenProto) {
   return CSSPageRule_Binding::Wrap(aCx, this, aGivenProto);

layout/style/CSSPageRule.h

@@ -12,6 +12,9 @@
 
 #include "nsDOMCSSDeclaration.h"
 #include "nsICSSDeclaration.h"
+// TODO alaskanemily: This is only needed by the generated bindings.
+// Ideally it would only be included from there.
+#include "nsCSSProps.h"
 
 namespace mozilla {
 class DeclarationBlock;
@@ -28,6 +31,8 @@ class CSSPageRuleDeclaration final : public nsDOMCSSDeclaration {
   nsINode* GetAssociatedNode() const final;
   nsISupports* GetParentObject() const final;
 
+  JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final;
+
  protected:
   DeclarationBlock* GetOrCreateCSSDeclaration(
       Operation aOperation, DeclarationBlock** aCreated) final;
@@ -69,7 +74,7 @@ class CSSPageRule final : public css::GroupRule {
   // WebIDL interfaces
   StyleCssRuleType Type() const final;
   void GetCssText(nsACString& aCssText) const final;
-  nsICSSDeclaration* Style();
+  CSSPageRuleDeclaration* Style() { return &mDecls; }
 
   void GetSelectorText(nsACString& aSelectorText) const;
   void SetSelectorText(const nsACString& aSelectorText);

servo/components/style/properties/longhands/margin.mako.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
-<% from data import ALL_SIDES, DEFAULT_RULES_AND_PAGE, maybe_moz_logical_alias %>
+<% from data import ALL_SIDES, DEFAULT_RULES, DEFAULT_RULES_AND_PAGE, maybe_moz_logical_alias %>
 
 % for side in ALL_SIDES:
     <%
@@ -22,7 +22,7 @@
         logical=side[1],
         logical_group="margin",
         spec=spec,
-        rule_types_allowed=DEFAULT_RULES_AND_PAGE,
+        rule_types_allowed=DEFAULT_RULES if side[1] else DEFAULT_RULES_AND_PAGE,
         servo_restyle_damage="reflow",
         affects="layout",
     )}

testing/web-platform/meta/css/cssom/idlharness.html.ini

@@ -170,54 +170,6 @@
   [CSSImportRule interface: sheet.cssRules[0\] must inherit property "supportsText" with the proper type]
     expected: FAIL
 
-  [CSSPageDescriptors interface: existence and properties of interface object]
-    expected: FAIL
-
-  [CSSPageDescriptors interface object length]
-    expected: FAIL
-
-  [CSSPageDescriptors interface object name]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: existence and properties of interface prototype object]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: existence and properties of interface prototype object's "constructor" property]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: existence and properties of interface prototype object's @@unscopables property]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute margin]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute marginTop]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute marginRight]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute marginBottom]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute marginLeft]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute margin-top]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute margin-right]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute margin-bottom]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute margin-left]
-    expected: FAIL
-
-  [CSSPageDescriptors interface: attribute size]
-    expected: FAIL
-
   [CSSPageDescriptors interface: attribute marks]
     expected: FAIL
 

testing/web-platform/tests/css/cssom/page-descriptors.html

@@ -3,9 +3,22 @@
 <head>
   <link rel="author" title="Mozilla" href="https://mozilla.org">
   <link rel="help" href="https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface">
-  <title>Page descriptors shouldn't be exposed to CSS Style declarations</title>
+  <title>CSSPageDescriptors properties tests</title>
   <script src="/resources/testharness.js"></script>
   <script src="/resources/testharnessreport.js"></script>
+  <style>
+@page {
+    size: a3;
+    page-orientation: rotate-right;
+    margin: 1em 24px 2in 101.5mm;
+}
+@page {
+    size: jis-b5 landscape;
+}
+@page {
+    size: 216mm;
+}
+  </style>
 </head>
 <body>
 <div id="target"></div>
@@ -15,8 +28,16 @@
 let element = document.getElementById("target");
 let computedStyle = window.getComputedStyle(element);
 let style = element.style;
+let styleSheet = document.styleSheets[0];
+let marginNames = ["left", "right", "top", "bottom"];
+let pageDescriptors = ["margin", "page-orientation", "size"];
+marginNames.forEach(function(n){
+    pageDescriptors.push("margin-" + n);
+    pageDescriptors.push("margin" + n[0].toUpperCase() + n.slice(1));
+});
 
 test(t => {
+    // Check that size isn't exposed on all CSS style declarations.
     assert_equals(computedStyle.size, undefined,
         "computed style should not have size property");
     assert_equals(computedStyle.getPropertyValue("size"), "",
@@ -34,6 +55,63 @@ test(t => {
         assert_equals(style.getPropertyValue("size"), "",
             "style getPropertyValue(\"size\") should be empty after assigning size=" + val);
     }
+    pageDescriptors.forEach(function(prop){
+        assert_own_property(styleSheet.cssRules[0].style.__proto__, prop,
+            "CSSPageDescriptors should have property " + prop);
+    });
+    assert_equals(styleSheet.cssRules[0].style.size, "a3");
+    assert_equals(styleSheet.cssRules[0].style.pageOrientation, "rotate-right");
+    assert_equals(styleSheet.cssRules[0].style.getPropertyValue("page-orientation"), "rotate-right",
+        'Value of page-orientation should match pageOrientation from CSS');
+    assert_equals(styleSheet.cssRules[1].style.size, "jis-b5 landscape");
+    assert_equals(styleSheet.cssRules[2].style.size, "216mm");
+
+    // Ensure we can set the size property to a valid value.
+    styleSheet.cssRules[2].style.size = "portrait";
+    assert_equals(styleSheet.cssRules[2].style.size, "portrait",
+        'Should have been able to set size property to "portrait" on CSSPageDescriptors');
+    // Ensure we cannot set the size property to an invalid property.
+    styleSheet.cssRules[2].style.size = "notarealsize";
+    assert_equals(styleSheet.cssRules[2].style.size, "portrait",
+        'Should not have been able to set size property to "notarealsize" on CSSPageDescriptors');
+
+    // Ensure we can set the orientation property to a valid value.
+    styleSheet.cssRules[2].style.pageOrientation = "rotate-left";
+    assert_equals(styleSheet.cssRules[2].style.pageOrientation, "rotate-left",
+        'Should have been able to set pageOrientation property to "rotate-left" on CSSPageDescriptors');
+    assert_equals(styleSheet.cssRules[2].style.getPropertyValue("page-orientation"), "rotate-left",
+        'Value of page-orientation should match pageOrientation after setting from script');
+    // Ensure we cannot set the orientation property to an invalid property.
+    styleSheet.cssRules[2].style.pageOrientation = "schmotate-schmeft";
+    assert_equals(styleSheet.cssRules[2].style.pageOrientation, "rotate-left",
+        'Should not have been able to set pageOrientation property to "schmotate-schmeft" on CSSPageDescriptors');
+
+    // Ensure we cannot set invalid page properties.
+    styleSheet.cssRules[2].style.setProperty("float", "left");
+    assert_equals(styleSheet.cssRules[2].style.cssFloat, undefined);
+
+    assert_equals(styleSheet.cssRules[0].style.marginLeft, "101.5mm");
+    assert_equals(styleSheet.cssRules[0].style.marginRight, "24px");
+    assert_equals(styleSheet.cssRules[0].style.marginTop, "1em");
+    assert_equals(styleSheet.cssRules[0].style.marginBottom, "2in");
+    marginNames.forEach(function(name){
+        let name1 = "margin-" + name;
+        let name2 = "margin" + name[0].toUpperCase() + name.slice(1);
+        assert_equals(styleSheet.cssRules[0].style[name1],
+            styleSheet.cssRules[0].style[name2],
+            "CSSPageDescriptors " + name1 + " and " + name2 + " should be the same.");
+        // Attempt setting through each name and ensure it is represented in the other.
+        styleSheet.cssRules[0].style[name1] = "99px";
+        assert_equals(styleSheet.cssRules[0].style[name1], "99px",
+            "Should have been able to set " + name1 + " property on CSSPageDescriptors");
+        assert_equals(styleSheet.cssRules[0].style[name2], "99px",
+            "Setting " + name1 + " on CSSPageDescriptors should also set " + name2);
+        styleSheet.cssRules[0].style[name2] = "216px";
+        assert_equals(styleSheet.cssRules[0].style[name2], "216px",
+            "Should have been able to set " + name2 + " property on CSSPageDescriptors");
+        assert_equals(styleSheet.cssRules[0].style[name1], "216px",
+            "Setting " + name2 + " on CSSPageDescriptors should also set " + name1);
+    });
 });
 </script>
 </body>