Fix two coding mistakes in MathMLInlineContainerElement::childrenChanged

https://bugs.webkit.org/show_bug.cgi?id=156538

Patch by Frederic Wang <fwang@igalia.com> on 2016-04-13
Reviewed by Darin Adler.

We fix the call to updateOperatorProperties inside MathMLInlineContainerElement::childrenChanged
for the <math> and <msqrt> tags.

The <math> tag is already a RenderMathMLRow so the hasTagName(mathTag)
conditional is never executed. The tag does not create any anonymous
wrapper so we do not need a special case for it anyway.

The <msqrt> tag is not a RenderMathMLRow (yet). However, the anonymous
wrapper behaving as a RenderMathMLRow is actually the last child, not
the first one.

No new tests, this is already covered by mathml/presentation/mo-form-dynamic.html
Note that for some reason the coding error for <msqrt> only shows up
after the refactoring of bug 152244.

* mathml/MathMLInlineContainerElement.cpp:
(WebCore::MathMLInlineContainerElement::childrenChanged): Fix the two mistakes and add some FIXME comments.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@199497 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fecb9e1..4a7e9c3 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2016-04-13  Frederic Wang  <fwang@igalia.com>
+
+        Fix two coding mistakes in MathMLInlineContainerElement::childrenChanged
+        https://bugs.webkit.org/show_bug.cgi?id=156538
+
+        Reviewed by Darin Adler.
+
+        We fix the call to updateOperatorProperties inside MathMLInlineContainerElement::childrenChanged
+        for the <math> and <msqrt> tags.
+
+        The <math> tag is already a RenderMathMLRow so the hasTagName(mathTag)
+        conditional is never executed. The tag does not create any anonymous
+        wrapper so we do not need a special case for it anyway.
+
+        The <msqrt> tag is not a RenderMathMLRow (yet). However, the anonymous
+        wrapper behaving as a RenderMathMLRow is actually the last child, not
+        the first one.
+
+        No new tests, this is already covered by mathml/presentation/mo-form-dynamic.html
+        Note that for some reason the coding error for <msqrt> only shows up
+        after the refactoring of bug 152244.
+
+        * mathml/MathMLInlineContainerElement.cpp:
+        (WebCore::MathMLInlineContainerElement::childrenChanged): Fix the two mistakes and add some FIXME comments.
+
 2016-04-12  Chris Dumez  <cdumez@apple.com>
 
         Attr.value should not be nullable

diff --git a/Source/WebCore/mathml/MathMLInlineContainerElement.cpp b/Source/WebCore/mathml/MathMLInlineContainerElement.cpp
index 8011b5e..7eb1222 100644 (file)
--- a/Source/WebCore/mathml/MathMLInlineContainerElement.cpp
+++ b/Source/WebCore/mathml/MathMLInlineContainerElement.cpp
@@ -58,10 +58,15 @@ Ref<MathMLInlineContainerElement> MathMLInlineContainerElement::create(const Qua
 void MathMLInlineContainerElement::childrenChanged(const ChildChange& change)
 {
     if (renderer()) {
+        // FIXME: Parsing of operator properties should be done in the element classes rather than in the renderer classes.
+        // See http://webkit.org/b/156537
         if (is<RenderMathMLRow>(*renderer()))
             downcast<RenderMathMLRow>(*renderer()).updateOperatorProperties();
-        else if (hasTagName(mathTag) || hasTagName(msqrtTag)) {
-            auto* childRenderer = renderer()->firstChild();
+        else if (hasTagName(msqrtTag)) {
+            // Update operator properties for the base wrapper.
+            // FIXME: This won't be necessary when RenderMathMLSquareRoot derives from RenderMathMLRow and does not use anonymous wrappers.
+            // See http://webkit.org/b/153987
+            auto* childRenderer = renderer()->lastChild();
             if (is<RenderMathMLRow>(childRenderer))
                 downcast<RenderMathMLRow>(*childRenderer).updateOperatorProperties();
         }