Parsing of HTML tags in MathML Text Insertion Points leads to bogus parser behavior
authoradamk@chromium.org <adamk@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Feb 2013 01:05:07 +0000 (01:05 +0000)
committeradamk@chromium.org <adamk@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Feb 2013 01:05:07 +0000 (01:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110808

Reviewed by Adam Barth.

Source/WebCore:

When looking for various table tags in the HTMLElementStack, compare
QualifiedNames rather than just local names, where necessary.

Note that not all uses have been "fixed"; I've only changed for which
I could write a test with differing behavior. A followup patch to
rationalize the use of QualifiedName vs local names would be ideal.

Tests: html5lib/generated/run-math-data.html
       html5lib/generated/run-math-write.html

* html/parser/HTMLElementStack.cpp:
(WebCore::inScopeCommon): Added a version of inScopeCommon that
handles QualifiedNames instead of just localNames.
(WebCore::HTMLElementStack::inTableScope): When given a QualifiedName,
call the new version of inScopeCommon().
* html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processStartTag):
(WebCore::HTMLTreeBuilder::processEndTagForInTableBody):
(WebCore::HTMLTreeBuilder::processTrEndTagForInRow):

LayoutTests:

* html5lib/generated/run-math-data-expected.txt: Added.
* html5lib/generated/run-math-data.html: Added.
* html5lib/generated/run-math-write-expected.txt: Added.
* html5lib/generated/run-math-write.html: Added.
* html5lib/resources/math.dat: Added.

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

LayoutTests/ChangeLog
LayoutTests/html5lib/generated/run-math-data-expected.txt [new file with mode: 0644]
LayoutTests/html5lib/generated/run-math-data.html [new file with mode: 0644]
LayoutTests/html5lib/generated/run-math-write-expected.txt [new file with mode: 0644]
LayoutTests/html5lib/generated/run-math-write.html [new file with mode: 0644]
LayoutTests/html5lib/resources/math.dat [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/parser/HTMLElementStack.cpp
Source/WebCore/html/parser/HTMLTreeBuilder.cpp

index 6a31807..da1a4c7 100644 (file)
@@ -1,3 +1,16 @@
+2013-02-26  Adam Klein  <adamk@chromium.org>
+
+        Parsing of HTML tags in MathML Text Insertion Points leads to bogus parser behavior
+        https://bugs.webkit.org/show_bug.cgi?id=110808
+
+        Reviewed by Adam Barth.
+
+        * html5lib/generated/run-math-data-expected.txt: Added.
+        * html5lib/generated/run-math-data.html: Added.
+        * html5lib/generated/run-math-write-expected.txt: Added.
+        * html5lib/generated/run-math-write.html: Added.
+        * html5lib/resources/math.dat: Added.
+
 2013-02-26  Kaustubh Atrawalkar  <kaustubh@motorola.com>
 
         Notification.requestPermission callback should be optional
diff --git a/LayoutTests/html5lib/generated/run-math-data-expected.txt b/LayoutTests/html5lib/generated/run-math-data-expected.txt
new file mode 100644 (file)
index 0000000..0c3b483
--- /dev/null
@@ -0,0 +1 @@
+../resources/math.dat: PASS
diff --git a/LayoutTests/html5lib/generated/run-math-data.html b/LayoutTests/html5lib/generated/run-math-data.html
new file mode 100644 (file)
index 0000000..2b25862
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<script>
+var test_files = [ '../resources/math.dat' ]
+</script>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>window.forceDataURLs = true;</script>
+<script src="../resources/runner.js"></script>
diff --git a/LayoutTests/html5lib/generated/run-math-write-expected.txt b/LayoutTests/html5lib/generated/run-math-write-expected.txt
new file mode 100644 (file)
index 0000000..0c3b483
--- /dev/null
@@ -0,0 +1 @@
+../resources/math.dat: PASS
diff --git a/LayoutTests/html5lib/generated/run-math-write.html b/LayoutTests/html5lib/generated/run-math-write.html
new file mode 100644 (file)
index 0000000..b910334
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<script>
+var test_files = [ '../resources/math.dat' ]
+</script>
+<script src="../../resources/dump-as-markup.js"></script>
+
+<script src="../resources/runner.js"></script>
diff --git a/LayoutTests/html5lib/resources/math.dat b/LayoutTests/html5lib/resources/math.dat
new file mode 100644 (file)
index 0000000..ae9cd7c
--- /dev/null
@@ -0,0 +1,81 @@
+#data
+<math><tr><td><mo><tr>
+#errors
+#document-fragment
+td
+#document
+| <math math>
+|   <math tr>
+|     <math td>
+|       <math mo>
+
+#data
+<math><tr><td><mo><tr>
+#errors
+#document-fragment
+tr
+#document
+| <math math>
+|   <math tr>
+|     <math td>
+|       <math mo>
+
+#data
+<math><thead><mo><tbody>
+#errors
+#document-fragment
+thead
+#document
+| <math math>
+|   <math thead>
+|     <math mo>
+
+#data
+<math><tfoot><mo><tbody>
+#errors
+#document-fragment
+tfoot
+#document
+| <math math>
+|   <math tfoot>
+|     <math mo>
+
+#data
+<math><tbody><mo><tfoot>
+#errors
+#document-fragment
+tbody
+#document
+| <math math>
+|   <math tbody>
+|     <math mo>
+
+#data
+<math><tbody><mo></table>
+#errors
+#document-fragment
+tbody
+#document
+| <math math>
+|   <math tbody>
+|     <math mo>
+
+#data
+<math><thead><mo></table>
+#errors
+#document-fragment
+tbody
+#document
+| <math math>
+|   <math thead>
+|     <math mo>
+
+#data
+<math><tfoot><mo></table>
+#errors
+#document-fragment
+tbody
+#document
+| <math math>
+|   <math tfoot>
+|     <math mo>
index df44cbd..cdf24d5 100644 (file)
@@ -1,3 +1,30 @@
+2013-02-26  Adam Klein  <adamk@chromium.org>
+
+        Parsing of HTML tags in MathML Text Insertion Points leads to bogus parser behavior
+        https://bugs.webkit.org/show_bug.cgi?id=110808
+
+        Reviewed by Adam Barth.
+
+        When looking for various table tags in the HTMLElementStack, compare
+        QualifiedNames rather than just local names, where necessary.
+
+        Note that not all uses have been "fixed"; I've only changed for which
+        I could write a test with differing behavior. A followup patch to
+        rationalize the use of QualifiedName vs local names would be ideal.
+
+        Tests: html5lib/generated/run-math-data.html
+               html5lib/generated/run-math-write.html
+
+        * html/parser/HTMLElementStack.cpp:
+        (WebCore::inScopeCommon): Added a version of inScopeCommon that
+        handles QualifiedNames instead of just localNames.
+        (WebCore::HTMLElementStack::inTableScope): When given a QualifiedName,
+        call the new version of inScopeCommon().
+        * html/parser/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::processStartTag):
+        (WebCore::HTMLTreeBuilder::processEndTagForInTableBody):
+        (WebCore::HTMLTreeBuilder::processTrEndTagForInRow):
+
 2013-02-26  Kaustubh Atrawalkar  <kaustubh@motorola.com>
 
         Notification.requestPermission callback should be optional
index b804677..50097ad 100644 (file)
@@ -459,6 +459,20 @@ bool inScopeCommon(HTMLElementStack::ElementRecord* top, const AtomicString& tar
     return false;
 }
 
+template <bool isMarker(HTMLStackItem*)>
+bool inScopeCommon(HTMLElementStack::ElementRecord* top, const QualifiedName& targetTag)
+{
+    for (HTMLElementStack::ElementRecord* pos = top; pos; pos = pos->next()) {
+        HTMLStackItem* item = pos->stackItem().get();
+        if (item->hasTagName(targetTag))
+            return true;
+        if (isMarker(item))
+            return false;
+    }
+    ASSERT_NOT_REACHED(); // <html> is always on the stack and is a scope marker.
+    return false;
+}
+
 bool HTMLElementStack::hasNumberedHeaderElementInScope() const
 {
     for (ElementRecord* record = m_top.get(); record; record = record->next()) {
@@ -514,8 +528,7 @@ bool HTMLElementStack::inTableScope(const AtomicString& targetTag) const
 
 bool HTMLElementStack::inTableScope(const QualifiedName& tagName) const
 {
-    // FIXME: Is localName() right for non-html elements?
-    return inTableScope(tagName.localName());
+    return inScopeCommon<isTableScopeMarker>(m_top.get(), tagName);
 }
 
 bool HTMLElementStack::inButtonScope(const AtomicString& targetTag) const
index 798f05e..55c110e 100644 (file)
@@ -1234,7 +1234,7 @@ void HTMLTreeBuilder::processStartTag(AtomicHTMLToken* token)
         }
         if (isCaptionColOrColgroupTag(token->name()) || isTableBodyContextTag(token->name())) {
             // FIXME: This is slow.
-            if (!m_tree.openElements()->inTableScope(tbodyTag.localName()) && !m_tree.openElements()->inTableScope(theadTag.localName()) && !m_tree.openElements()->inTableScope(tfootTag.localName())) {
+            if (!m_tree.openElements()->inTableScope(tbodyTag) && !m_tree.openElements()->inTableScope(theadTag) && !m_tree.openElements()->inTableScope(tfootTag)) {
                 ASSERT(isParsingFragmentOrTemplateContents());
                 parseError(token);
                 return;
@@ -1723,7 +1723,7 @@ void HTMLTreeBuilder::processEndTagForInTableBody(AtomicHTMLToken* token)
     }
     if (token->name() == tableTag) {
         // FIXME: This is slow.
-        if (!m_tree.openElements()->inTableScope(tbodyTag.localName()) && !m_tree.openElements()->inTableScope(theadTag.localName()) && !m_tree.openElements()->inTableScope(tfootTag.localName())) {
+        if (!m_tree.openElements()->inTableScope(tbodyTag) && !m_tree.openElements()->inTableScope(theadTag) && !m_tree.openElements()->inTableScope(tfootTag)) {
             ASSERT(isParsingFragmentOrTemplateContents());
             parseError(token);
             return;
@@ -1978,7 +1978,7 @@ bool HTMLTreeBuilder::processCaptionEndTagForInCaption()
 
 bool HTMLTreeBuilder::processTrEndTagForInRow()
 {
-    if (!m_tree.openElements()->inTableScope(trTag.localName())) {
+    if (!m_tree.openElements()->inTableScope(trTag)) {
         ASSERT(isParsingFragmentOrTemplateContents());
         // FIXME: parse error
         return false;