Title: [204037] trunk
Revision
204037
Author
fred.w...@free.fr
Date
2016-08-02 12:53:57 -0700 (Tue, 02 Aug 2016)

Log Message

Move parsing of operator length attributes to MathMLOperatorElement
https://bugs.webkit.org/show_bug.cgi?id=160301

Patch by Frederic Wang <fw...@igalia.com> on 2016-08-02
Reviewed by Darin Adler.

Source/WebCore:

We move parsing of operator attributes lspace, rspace minsize, maxsize into the class
MathMLOperatorElement. We rely on the existing MathMLElement::Length structure and expand
it to handle the "infinity" case of maxsize that we always resolve as intMaxForLayoutUnit.
We also improve the resolution of a negative value for minsize and maxsize since the MathML
recommendation says it should be rounded up to the nearest valid value (which is zero)
instead of ignoring the attribute and using the default value. This is similar to r203285
for mfrac@linethickness. We also update the existing test for minsize/maxsize to take into
account that change.

No new tests, already covered by existing tests.

* mathml/MathMLElement.h: Add an "Infinity" type for MathML Length.
* mathml/MathMLOperatorElement.cpp:
(WebCore::MathMLOperatorElement::leadingSpace): Helper function to retrieve the cached value
for the lspace attribute, parse it if necessary.
(WebCore::MathMLOperatorElement::trailingSpace): Ditto for rspace.
(WebCore::MathMLOperatorElement::minSize): Ditto for minsize.
(WebCore::MathMLOperatorElement::maxSize): Ditto for maxsize. This attribute also accepts the
"infinity" value.
(WebCore::MathMLOperatorElement::parseAttribute): Mark attributes dirty.
* mathml/MathMLOperatorElement.h: Declare Length members and accessors.
* rendering/mathml/RenderMathMLBlock.cpp:
(WebCore::toUserUnits): Resolve Infinity as intMaxForLayoutUnit.
* rendering/mathml/RenderMathMLOperator.cpp:
(WebCore::RenderMathMLOperator::setOperatorProperties): We use toUserUnits to resolve the
lengths. Negative maxsize and minsize values now fallback to 0 instead of their default
values. We also remove the !isAnonymous() check since setOperatorProperties is overriden
in the RenderMathMLFencedOperator class.

LayoutTests:

* mathml/presentation/stretchy-minsize-maxsize-expected.html: Update the test for negative
minsize and maxsize to align on the behavior suggested in the MathML recommendation.
* mathml/presentation/stretchy-minsize-maxsize.html: Ditto.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (204036 => 204037)


--- trunk/LayoutTests/ChangeLog	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/LayoutTests/ChangeLog	2016-08-02 19:53:57 UTC (rev 204037)
@@ -1,3 +1,14 @@
+2016-08-02  Frederic Wang  <fw...@igalia.com>
+
+        Move parsing of operator length attributes to MathMLOperatorElement
+        https://bugs.webkit.org/show_bug.cgi?id=160301
+
+        Reviewed by Darin Adler.
+
+        * mathml/presentation/stretchy-minsize-maxsize-expected.html: Update the test for negative
+        minsize and maxsize to align on the behavior suggested in the MathML recommendation.
+        * mathml/presentation/stretchy-minsize-maxsize.html: Ditto.
+
 2016-08-02  Chris Dumez  <cdu...@apple.com>
 
         [WebIDL] Implement overload resolution algorithm

Modified: trunk/LayoutTests/mathml/presentation/stretchy-minsize-maxsize-expected.html (204036 => 204037)


--- trunk/LayoutTests/mathml/presentation/stretchy-minsize-maxsize-expected.html	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/LayoutTests/mathml/presentation/stretchy-minsize-maxsize-expected.html	2016-08-02 19:53:57 UTC (rev 204037)
@@ -43,13 +43,13 @@
 
       <math>
         <mrow>
-          <mo symmetric="false">|</mo>
+          <mo symmetric="false" minsize="0em">|</mo>
           <mspace height="4em" depth="4em"/>
         </mrow>
       </math>
       <math>
         <mrow>
-          <mo symmetric="false">|</mo>
+          <mo symmetric="false" maxsize="0em">|</mo>
           <mspace height="4em" depth="4em"/>
         </mrow>
       </math>

Modified: trunk/LayoutTests/mathml/presentation/stretchy-minsize-maxsize.html (204036 => 204037)


--- trunk/LayoutTests/mathml/presentation/stretchy-minsize-maxsize.html	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/LayoutTests/mathml/presentation/stretchy-minsize-maxsize.html	2016-08-02 19:53:57 UTC (rev 204037)
@@ -45,7 +45,7 @@
         </mrow>
       </math>
 
-      <!-- We verify that negative values are ignored. -->
+      <!-- We verify that negative values are treated as 0. -->
       <math>
         <mrow>
           <mo symmetric="false" minsize="-5em">|</mo>

Modified: trunk/Source/WebCore/ChangeLog (204036 => 204037)


--- trunk/Source/WebCore/ChangeLog	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/Source/WebCore/ChangeLog	2016-08-02 19:53:57 UTC (rev 204037)
@@ -1,3 +1,39 @@
+2016-08-02  Frederic Wang  <fw...@igalia.com>
+
+        Move parsing of operator length attributes to MathMLOperatorElement
+        https://bugs.webkit.org/show_bug.cgi?id=160301
+
+        Reviewed by Darin Adler.
+
+        We move parsing of operator attributes lspace, rspace minsize, maxsize into the class
+        MathMLOperatorElement. We rely on the existing MathMLElement::Length structure and expand
+        it to handle the "infinity" case of maxsize that we always resolve as intMaxForLayoutUnit.
+        We also improve the resolution of a negative value for minsize and maxsize since the MathML
+        recommendation says it should be rounded up to the nearest valid value (which is zero)
+        instead of ignoring the attribute and using the default value. This is similar to r203285
+        for mfrac@linethickness. We also update the existing test for minsize/maxsize to take into
+        account that change.
+
+        No new tests, already covered by existing tests.
+
+        * mathml/MathMLElement.h: Add an "Infinity" type for MathML Length.
+        * mathml/MathMLOperatorElement.cpp:
+        (WebCore::MathMLOperatorElement::leadingSpace): Helper function to retrieve the cached value
+        for the lspace attribute, parse it if necessary.
+        (WebCore::MathMLOperatorElement::trailingSpace): Ditto for rspace.
+        (WebCore::MathMLOperatorElement::minSize): Ditto for minsize.
+        (WebCore::MathMLOperatorElement::maxSize): Ditto for maxsize. This attribute also accepts the
+        "infinity" value.
+        (WebCore::MathMLOperatorElement::parseAttribute): Mark attributes dirty.
+        * mathml/MathMLOperatorElement.h: Declare Length members and accessors.
+        * rendering/mathml/RenderMathMLBlock.cpp:
+        (WebCore::toUserUnits): Resolve Infinity as intMaxForLayoutUnit.
+        * rendering/mathml/RenderMathMLOperator.cpp:
+        (WebCore::RenderMathMLOperator::setOperatorProperties): We use toUserUnits to resolve the
+        lengths. Negative maxsize and minsize values now fallback to 0 instead of their default
+        values. We also remove the !isAnonymous() check since setOperatorProperties is overriden
+        in the RenderMathMLFencedOperator class.
+
 2016-08-02  Frederic Wang  <fred.w...@free.fr>
 
         Unreviewed build error fix.

Modified: trunk/Source/WebCore/mathml/MathMLElement.h (204036 => 204037)


--- trunk/Source/WebCore/mathml/MathMLElement.h	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/Source/WebCore/mathml/MathMLElement.h	2016-08-02 19:53:57 UTC (rev 204037)
@@ -59,7 +59,7 @@
     // MathML lengths (https://www.w3.org/TR/MathML3/chapter2.html#fund.units)
     // TeX's Math Unit is used internally for named spaces (1 mu = 1/18 em).
     // Unitless values are interpreted as a multiple of a reference value.
-    enum class LengthType { Cm, Em, Ex, In, MathUnit, Mm, ParsingFailed, Pc, Percentage, Pt, Px, UnitLess };
+    enum class LengthType { Cm, Em, Ex, In, MathUnit, Mm, ParsingFailed, Pc, Percentage, Pt, Px, UnitLess, Infinity };
     struct Length {
         LengthType type { LengthType::ParsingFailed };
         float value { 0 };

Modified: trunk/Source/WebCore/mathml/MathMLOperatorElement.cpp (204036 => 204037)


--- trunk/Source/WebCore/mathml/MathMLOperatorElement.cpp	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/Source/WebCore/mathml/MathMLOperatorElement.cpp	2016-08-02 19:53:57 UTC (rev 204037)
@@ -179,6 +179,37 @@
     return space;
 }
 
+const MathMLElement::Length& MathMLOperatorElement::leadingSpace()
+{
+    return cachedMathMLLength(MathMLNames::lspaceAttr, m_leadingSpace);
+}
+
+const MathMLElement::Length& MathMLOperatorElement::trailingSpace()
+{
+    return cachedMathMLLength(MathMLNames::rspaceAttr, m_trailingSpace);
+}
+
+const MathMLElement::Length& MathMLOperatorElement::minSize()
+{
+    return cachedMathMLLength(MathMLNames::minsizeAttr, m_minSize);
+}
+
+const MathMLElement::Length& MathMLOperatorElement::maxSize()
+{
+    if (m_maxSize)
+        return m_maxSize.value();
+
+    const AtomicString& value = attributeWithoutSynchronization(MathMLNames::maxsizeAttr);
+    if (value == "infinity") {
+        Length maxsize;
+        maxsize.type = LengthType::Infinity;
+        m_maxSize = maxsize;
+    } else
+        m_maxSize = parseMathMLLength(value);
+
+    return m_maxSize.value();
+}
+
 void MathMLOperatorElement::childrenChanged(const ChildChange& change)
 {
     m_operatorText = Nullopt;
@@ -213,6 +244,14 @@
         m_properties.dirtyFlags = MathMLOperatorDictionary::allFlags;
     } else if (auto flag = attributeNameToPropertyFlag(name))
         m_properties.dirtyFlags |= flag.value();
+    else if (name == lspaceAttr)
+        m_leadingSpace = Nullopt;
+    else if (name == rspaceAttr)
+        m_trailingSpace = Nullopt;
+    else if (name == minsizeAttr)
+        m_minSize = Nullopt;
+    else if (name == maxsizeAttr)
+        m_maxSize = Nullopt;
 
     if ((name == stretchyAttr || name == lspaceAttr || name == rspaceAttr || name == movablelimitsAttr) && renderer()) {
         downcast<RenderMathMLOperator>(*renderer()).updateFromElement();

Modified: trunk/Source/WebCore/mathml/MathMLOperatorElement.h (204036 => 204037)


--- trunk/Source/WebCore/mathml/MathMLOperatorElement.h	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/Source/WebCore/mathml/MathMLOperatorElement.h	2016-08-02 19:53:57 UTC (rev 204037)
@@ -41,6 +41,10 @@
     bool hasProperty(MathMLOperatorDictionary::Flag);
     Length defaultLeadingSpace();
     Length defaultTrailingSpace();
+    const Length& leadingSpace();
+    const Length& trailingSpace();
+    const Length& minSize();
+    const Length& maxSize();
 
 private:
     MathMLOperatorElement(const QualifiedName& tagName, Document&);
@@ -68,6 +72,11 @@
     };
     OperatorProperties m_properties;
     void computeOperatorFlag(MathMLOperatorDictionary::Flag);
+
+    Optional<Length> m_leadingSpace;
+    Optional<Length> m_trailingSpace;
+    Optional<Length> m_minSize;
+    Optional<Length> m_maxSize;
 };
 
 }

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp (204036 => 204037)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp	2016-08-02 19:53:57 UTC (rev 204037)
@@ -165,6 +165,8 @@
         return referenceValue * length.value;
     case MathMLElement::LengthType::ParsingFailed:
         return referenceValue;
+    case MathMLElement::LengthType::Infinity:
+        return intMaxForLayoutUnit;
     default:
         ASSERT_NOT_REACHED();
         return referenceValue;

Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp (204036 => 204037)


--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp	2016-08-02 19:23:26 UTC (rev 204036)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp	2016-08-02 19:53:57 UTC (rev 204037)
@@ -79,22 +79,19 @@
     // We determine the stretch direction (default is vertical).
     m_isVertical = MathMLOperatorDictionary::isVertical(textContent());
 
-    // Initialize with the default values.
+    // FIXME: Negative leading spaces must be implemented (https://webkit.org/b/124830).
     m_leadingSpace = toUserUnits(element().defaultLeadingSpace(), style(), 0);
+    m_leadingSpace = std::max<LayoutUnit>(0, toUserUnits(element().leadingSpace(), style(), m_leadingSpace));
+
+    // FIXME: Negative trailing spaces must be implemented (https://webkit.org/b/124830).
     m_trailingSpace = toUserUnits(element().defaultTrailingSpace(), style(), 0);
-    m_minSize = style().fontCascade().size(); // This sets minsize to "1em".
-    m_maxSize = intMaxForLayoutUnit; // This sets maxsize to "infinity".
+    m_trailingSpace = std::max<LayoutUnit>(0, toUserUnits(element().trailingSpace(), style(), m_trailingSpace));
 
-    if (!isAnonymous()) {
-        // Finally, we make the attribute values override the default.
-        parseMathMLLength(element().attributeWithoutSynchronization(MathMLNames::lspaceAttr), m_leadingSpace, &style(), false); // FIXME: Negative leading space must be implemented (https://bugs.webkit.org/show_bug.cgi?id=124830).
-        parseMathMLLength(element().attributeWithoutSynchronization(MathMLNames::rspaceAttr), m_trailingSpace, &style(), false); // FIXME: Negative trailing space must be implemented (https://bugs.webkit.org/show_bug.cgi?id=124830).
+    m_minSize = style().fontCascade().size(); // Default minsize is "1em".
+    m_minSize = std::max<LayoutUnit>(0, toUserUnits(element().minSize(), style(), m_minSize));
 
-        parseMathMLLength(element().attributeWithoutSynchronization(MathMLNames::minsizeAttr), m_minSize, &style(), false);
-        const AtomicString& maxsize = element().attributeWithoutSynchronization(MathMLNames::maxsizeAttr);
-        if (maxsize != "infinity")
-            parseMathMLLength(maxsize, m_maxSize, &style(), false);
-    }
+    m_maxSize = intMaxForLayoutUnit; // Default maxsize is "infinity".
+    m_maxSize = std::max<LayoutUnit>(0, toUserUnits(element().maxSize(), style(), m_maxSize));
 }
 
 void RenderMathMLOperator::stretchTo(LayoutUnit heightAboveBaseline, LayoutUnit depthBelowBaseline)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to