Bug 1667527 - Implement CSS parsing for the math-depth property r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D91500
2020-09-28 10:42:49 +00:00 · 2020-09-28 10:42:49 +00:00 · 42b117ee83
--- a/dom/mathml/MathMLElement.cpp
+++ b/dom/mathml/MathMLElement.cpp
@ -448,18 +448,9 @@ void MathMLElement::MapMathMLAttributesInto(
      nsresult errorCode;
      int32_t intValue = str.ToInteger(&errorCode);
      if (NS_SUCCEEDED(errorCode)) {
-        // This is kind of cheesy ... if the scriptlevel has a sign,
-        // then it's a relative value and we store the nsCSSValue as an
-        // Integer to indicate that. Otherwise we store it as a Number
-        // to indicate that the scriptlevel is absolute.
-        // XXX Bug 1667090: Use math-depth: add(<integer>) for relative values
-        // and and math-depth: <integer> for absolute values.
        char16_t ch = str.CharAt(0);
-        if (ch == '+' || ch == '-') {
-          aDecls.SetIntValue(eCSSProperty_math_depth, intValue);
-        } else {
-          aDecls.SetNumberValue(eCSSProperty_math_depth, intValue);
-        }
+        bool isRelativeScriptLevel = (ch == '+' || ch == '-');
+        aDecls.SetMathDepthValue(intValue, isRelativeScriptLevel);
      } else {
        ReportParseErrorNoTag(str, nsGkAtoms::scriptlevel_, aDecls.Document());
      }
--- a/layout/mathml/mathml.css
+++ b/layout/mathml/mathml.css
@ -226,14 +226,14 @@ mtable[framespacing] > mtr > mtd {
   insufficient to control when the scriptlevel should be incremented. All other
   cases can be described using regular CSS, so we do it this way because it's
   more efficient and less code. */
-:-moz-math-increment-script-level { math-depth: +1; }
+:-moz-math-increment-script-level { math-depth: add(1); }

 /*
   The mfrac element sets displaystyle to "false", or if it was already false
   increments scriptlevel by 1, within numerator and denominator.
 */
 mfrac > * {
-    math-depth: auto;
+    math-depth: auto-add;
    math-style: compact;
 }

@ -243,7 +243,7 @@ mfrac > * {
   The msqrt element leaves both attributes unchanged within its argument.
 */
 mroot > :not(:first-child) {
-    math-depth: +2;
+    math-depth: add(2);
    math-style: compact;
 }

@ -267,7 +267,7 @@ msub > :not(:first-child),
 msup > :not(:first-child),
 msubsup > :not(:first-child),
 mmultiscripts > :not(:first-child) {
-    math-depth: +1;
+    math-depth: add(1);
    math-style: compact;
 }

@ -303,7 +303,7 @@ mtable { math-style: compact; }
   scriptlevel by 1, so the children are typically displayed in a smaller font.
   XXXfredw: This element is not implemented yet. See bug 534967.
 mscarries {
-  math-depth: +1;
+  math-depth: add(1);
  math-style: compact;
 }
 */
--- a/layout/style/MappedDeclarations.h
+++ b/layout/style/MappedDeclarations.h
@ -96,6 +96,11 @@ class MappedDeclarations final {
    Servo_DeclarationBlock_SetIntValue(mDecl, aId, aValue);
  }

+  // Set "math-depth: <integer>" or "math-depth: add(<integer>)"
+  void SetMathDepthValue(int32_t aValue, bool aIsRelative) {
+    Servo_DeclarationBlock_SetMathDepthValue(mDecl, aValue, aIsRelative);
+  }
+
  // Set "counter-reset: list-item <integer>".
  void SetCounterResetListItem(int32_t aValue) {
    Servo_DeclarationBlock_SetCounterResetListItem(mDecl, aValue);
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@ -13388,6 +13388,25 @@ if (IsCSSPropertyPrefEnabled("layout.css.aspect-ratio.enabled")) {
  };
 }

+if (IsCSSPropertyPrefEnabled("layout.css.math-depth.enabled")) {
+  gCSSProperties["math-depth"] = {
+    domProp: "mathDepth",
+    inherited: true,
+    type: CSS_TYPE_LONGHAND,
+    initial_values: ["0"],
+    other_values: [
+      "auto-add",
+      "123",
+      "-123",
+      "add(123)",
+      "add(-123)",
+      "calc(1 + 2*3)",
+      "add(calc(1 - 2/3))",
+    ],
+    invalid_values: ["auto", "1,23", "1.23", "add(1,23)", "add(1.23)"],
+  };
+}
+
 if (IsCSSPropertyPrefEnabled("layout.css.math-style.enabled")) {
  gCSSProperties["math-style"] = {
    domProp: "mathStyle",
--- a/servo/components/style/values/computed/font.rs
+++ b/servo/components/style/values/computed/font.rs
@ -35,6 +35,7 @@ use style_traits::{CssWriter, ParseError, ToCss};
 use to_shmem::{self, SharedMemoryBuilder, ToShmem};

 pub use crate::values::computed::Length as MozScriptMinSize;
+pub use crate::values::specified::Integer as SpecifiedInteger;
 pub use crate::values::specified::font::{FontSynthesis, MozScriptSizeMultiplier};
 pub use crate::values::specified::font::{XLang, XTextZoom};

@ -823,7 +824,7 @@ impl ToComputedValue for specified::MathDepth {
        use std::{cmp, i8};

        let int = match *self {
-            specified::MathDepth::Auto => {
+            specified::MathDepth::AutoAdd => {
                let parent = cx.builder.get_parent_font().clone_math_depth() as i32;
                let style = cx.builder.get_parent_font().clone_math_style();
                if style == MathStyleValue::Compact {
@ -832,17 +833,18 @@ impl ToComputedValue for specified::MathDepth {
                    parent
                }
            },
-            specified::MathDepth::Relative(rel) => {
+            specified::MathDepth::Add(rel) => {
                let parent = cx.builder.get_parent_font().clone_math_depth();
-                parent as i32 + rel
+                parent as i32 + rel.to_computed_value(cx)
            },
-            specified::MathDepth::MozAbsolute(abs) => abs,
+            specified::MathDepth::Absolute(abs) => abs.to_computed_value(cx),
        };
        cmp::min(int, i8::MAX as i32) as i8
    }

    fn from_computed_value(other: &i8) -> Self {
-        specified::MathDepth::MozAbsolute(*other as i32)
+        let computed_value = *other as i32;
+        specified::MathDepth::Absolute(SpecifiedInteger::from_computed_value(&computed_value))
    }
 }

--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@ -2296,38 +2296,36 @@ impl Parse for MozScriptMinSize {
    }
 }

+/// A value for the `math-depth` property.
+/// https://mathml-refresh.github.io/mathml-core/#the-math-script-level-property
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
 #[derive(Clone, Copy, Debug, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
-/// Changes the scriptlevel in effect for the children.
-/// Ref: https://wiki.mozilla.org/MathML:mstyle
-///
-/// The main effect of scriptlevel is to control the font size.
-/// https://www.w3.org/TR/MathML3/chapter3.html#presm.scriptlevel
 pub enum MathDepth {
-    /// Change `font-size` relatively.
-    Relative(i32),
-    /// Change `font-size` absolutely.
-    ///
-    /// Should only be serialized by presentation attributes, so even though
-    /// serialization for this would look the same as for the `Relative`
-    /// variant, it is unexposed, so no big deal.
+    /// Increment math-depth if math-style is compact.
+    AutoAdd,
+
+    /// Add the function's argument to math-depth.
    #[css(function)]
-    MozAbsolute(i32),
-    /// Change `font-size` automatically.
-    Auto,
+    Add(Integer),
+
+    /// Set math-depth to the specified value.
+    Absolute(Integer),
 }

 impl Parse for MathDepth {
    fn parse<'i, 't>(
-        _: &ParserContext,
+        context: &ParserContext,
        input: &mut Parser<'i, 't>,
    ) -> Result<MathDepth, ParseError<'i>> {
-        // We don't bother to handle calc here.
-        if let Ok(i) = input.try_parse(|i| i.expect_integer()) {
-            return Ok(MathDepth::Relative(i));
+        if input.try_parse(|i| i.expect_ident_matching("auto-add")).is_ok() {
+            return Ok(MathDepth::AutoAdd);
        }
-        input.expect_ident_matching("auto")?;
-        Ok(MathDepth::Auto)
+        if let Ok(math_depth_value) = input.try_parse(|input| Integer::parse(context, input)) {
+            return Ok(MathDepth::Absolute(math_depth_value));
+        }
+        input.expect_function_matching("add")?;
+        let math_depth_delta_value = input.parse_nested_block(|input| Integer::parse(context, input))?;
+        Ok(MathDepth::Add(math_depth_delta_value))
    }
 }

--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@ -4901,21 +4901,38 @@ pub extern "C" fn Servo_DeclarationBlock_SetIntValue(
    property: nsCSSPropertyID,
    value: i32,
 ) {
-    use style::properties::longhands::math_depth::SpecifiedValue as MathDepth;
    use style::properties::PropertyDeclaration;
    use style::values::specified::Integer;

    let long = get_longhand_from_id!(property);
    let prop = match_wrap_declared! { long,
        XSpan => Integer::new(value),
-        // Gecko uses Integer values to signal that it is relative
-        MathDepth => MathDepth::Relative(value),
    };
    write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
        decls.push(prop, Importance::Normal);
    })
 }

+#[no_mangle]
+pub extern "C" fn Servo_DeclarationBlock_SetMathDepthValue(
+    declarations: &RawServoDeclarationBlock,
+    value: i32,
+    is_relative: bool,
+) {
+    use style::properties::longhands::math_depth::SpecifiedValue as MathDepth;
+    use style::properties::PropertyDeclaration;
+
+    let integer_value = style::values::specified::Integer::new(value);
+    let prop = PropertyDeclaration::MathDepth(if is_relative {
+        MathDepth::Add(integer_value)
+    } else {
+        MathDepth::Absolute(integer_value)
+    });
+    write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
+        decls.push(prop, Importance::Normal);
+    })
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_SetCounterResetListItem(
    declarations: &RawServoDeclarationBlock,
@ -5072,7 +5089,6 @@ pub extern "C" fn Servo_DeclarationBlock_SetNumberValue(
    property: nsCSSPropertyID,
    value: f32,
 ) {
-    use style::properties::longhands::math_depth::SpecifiedValue as MathDepth;
    use style::properties::longhands::_moz_script_size_multiplier::SpecifiedValue as MozScriptSizeMultiplier;
    use style::properties::PropertyDeclaration;

@ -5080,8 +5096,6 @@ pub extern "C" fn Servo_DeclarationBlock_SetNumberValue(

    let prop = match_wrap_declared! { long,
        MozScriptSizeMultiplier => MozScriptSizeMultiplier(value),
-        // Gecko uses Number values to signal that it is absolute
-        MathDepth => MathDepth::MozAbsolute(value as i32),
    };
    write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
        decls.push(prop, Importance::Normal);
--- a/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/dir.ini
+++ b/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/dir.ini
@ -1 +1 @@
-prefs: [layout.css.math-style.enabled: true]
+prefs: [layout.css.math-style.enabled: true, layout.css.math-depth.enabled: true]
--- a/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative.html.ini
+++ b/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative.html.ini
@ -1,16 +0,0 @@
-[math-script-level-001.tentative.html]
-  [Inherited values of math-depth]
-    expected: FAIL
-
-  [Specified math-depth: add(<integer>)]
-    expected: FAIL
-
-  [Specified math-depth: <integer>]
-    expected: FAIL
-
-  [Specified math-depth: auto-add]
-    expected: FAIL
-
-  [Initial value of math-depth]
-    expected: FAIL
-
--- a/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-002.tentative.html.ini
+++ b/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-002.tentative.html.ini
@ -1,19 +0,0 @@
-[math-script-level-002.tentative.html]
-  [<integer> ; starting from level 50]
-    expected: FAIL
-
-  [auto ; starting from level 7]
-    expected: FAIL
-
-  [auto]
-    expected: FAIL
-
-  [add(<integer>) ; starting from level 3]
-    expected: FAIL
-
-  [<integer>]
-    expected: FAIL
-
-  [add(<integer>)]
-    expected: FAIL
-
--- a/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-004.tentative.html.ini
+++ b/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-004.tentative.html.ini
@ -1,9 +1,6 @@
 [math-script-level-004.tentative.html]
  expected:
-    if (processor == "x86") and debug and fission: ["OK", "TIMEOUT"]
-  [No MATH table]
-    expected: FAIL
-
+    if (processor == "x86") and debug and fission: [OK, TIMEOUT]
  [scriptPercentScaleDown=80, scriptScriptPercentScaleDown=0]
    expected: FAIL

--- a/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-002.tentative.html.ini
+++ b/testing/web-platform/meta/css/css-fonts/math-script-level-and-math-style/math-script-level-auto-and-math-style-002.tentative.html.ini
@ -1,2 +0,0 @@
-[math-script-level-auto-and-math-style-002.tentative.html]
-  expected: FAIL
--- a/testing/web-platform/tests/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative.html
+++ b/testing/web-platform/tests/css/css-fonts/math-script-level-and-math-style/math-script-level-001.tentative.html
@ -34,6 +34,18 @@
              assert_equals(mathDepth("specifiedAdd10From5"), "15");
              assert_equals(mathDepth("specifiedAdd-15From5"), "-10");
          }, "Specified math-depth: add(<integer>)");
+          test(function() {
+              assert_equals(mathDepth("invalidKeywordFrom3"), "3");
+              assert_equals(mathDepth("invalidFloatFrom3"), "3");
+              assert_equals(mathDepth("invalidCalcFrom3"), "3");
+              assert_equals(mathDepth("invalidAddCalcFrom3"), "3");
+              assert_equals(mathDepth("invalidAddFloatFrom3"), "3");
+          }, "Specified math-depth: invalid expressions");
+          test(function() {
+              const cssVariable = 3;
+              assert_equals(mathDepth("specifiedCalcFrom9"), `${Math.round(cssVariable/2)+10}`);
+              assert_equals(mathDepth("specifiedAddCalcFrom9"), `${9+(3*4-5)}`);
+          }, "Specified math-depth: calc() expressions");
          done();
      });
    </script>
@ -60,5 +72,16 @@
      <div id="specifiedAdd10From5" style="math-depth: add(10)"></div>
      <div id="specifiedAdd-15From5" style="math-depth: add(-15)"></div>
    </div>
+    <div style="math-depth: 3;">
+      <div id="invalidKeywordFrom3" style="math-depth: auto"></div>
+      <div id="invalidFloatFrom3" style="math-depth: 3.14"></div>
+      <div id="invalidCalcFrom3" style="math-depth: 1,2"></div>
+      <div id="invalidAddCalcFrom3" style="math-depth: add(3,4)"></div>
+      <div id="invalidAddFloatFrom3" style="math-depth: add(3.14)"></div>
+    </div>
+    <div style="math-depth: 9;">
+      <div id="specifiedCalcFrom9" style="--css-variable: 3; math-depth: calc(var(--css-variable)/2 + 10)"></div>
+      <div id="specifiedAddCalcFrom9" style="math-depth: add(calc(3*4 - 5))"></div>
+    </div>
  </body>
 </html>