Implement circa 2021 spec change for fragment parsing in foreign content

This change brings us into conformance with the “make breaking out of
foreign content apply in innerHTML” requirements from the HTML spec PR
at whatwg/html#6399 — and into interoperability
with current WebKit and Blink behaviour, making fragment parsing break
out of foreign content in the same way document parsing does.

Before this code change, when parsing elements like <p>, <div>, and
<nobr> inside SVG/MathML via innerHTML, they would stay in the foreign
namespace. But, with this change, they now correctly “break out”, and
are created in the HTML namespace.

TreeBuilder.java
----------------
  - Start tags: Remove fragment-only restriction on break-out behavior
  - End tags: Allow </p> and </br> to reach mode-handling-in-fragment case
  - Use currentPtr > 0 to preserve context element when popping

Author: sideshowbarker

src/nu/validator/htmlparser/impl/TreeBuilder.java

@@ -1531,12 +1531,19 @@ public final void startTag(ElementName elementName,
                             if (!(group == FONT && !(attributes.contains(AttributeName.COLOR)
                                     || attributes.contains(AttributeName.FACE) || attributes.contains(AttributeName.SIZE)))) {
                                 errHtmlStartTagInForeignContext(name);
-                                if (!fragment) {
-                                    while (!isSpecialParentInForeign(stack[currentPtr])) {
-                                        popForeign(-1, -1);
-                                    }
+                                // Pop until we reach an HTML namespace element,
+                                // HTML integration point, or MathML text integration point.
+                                // In fragment case, stop before popping the context element.
+                                while (currentPtr > 0 && !isSpecialParentInForeign(stack[currentPtr])) {
+                                    popForeign(-1, -1);
+                                }
+                                if (currentPtr > 0 || isSpecialParentInForeign(stack[currentPtr])) {
+                                    // Popped to an HTML element or integration point
                                     continue starttagloop;
-                                } // else fall thru
+                                }
+                                // In fragment case with foreign context, fall through
+                                // to let switch(mode) handle the token in HTML namespace
+                                break;
                             }
                             // CPPONLY: MOZ_FALLTHROUGH;
                         default:
@@ -3220,6 +3227,11 @@ public final void endTag(ElementName elementName) throws SAXException {
                 for (;;) {
                     if (eltPos == 0) {
                         assert fragment: "We can get this close to the root of the stack in foreign content only in the fragment case.";
+                        // For </p> and </br>, continue to mode handling
+                        // which will create implied start tags
+                        if (group == P || group == BR) {
+                            break; // break from inner loop, continue to switch(mode)
+                        }
                         break endtagloop;
                     }
                     if (stack[eltPos].name == name) {
@@ -3564,12 +3576,10 @@ public final void endTag(ElementName elementName) throws SAXException {
                             eltPos = findLastInButtonScope("p");
                             if (eltPos == TreeBuilder.NOT_FOUND_ON_STACK) {
                                 errNoElementToCloseButEndTagSeen("p");
-                                // XXX Can the 'in foreign' case happen anymore?
                                 if (isInForeign()) {
                                     errHtmlStartTagInForeignContext(name);
-                                    // Check for currentPtr for the fragment
-                                    // case.
-                                    while (currentPtr >= 0 && stack[currentPtr].ns != "http://www.w3.org/1999/xhtml") {
+                                    // Pop foreign elements, but keep context element in fragment case
+                                    while (currentPtr > 0 && stack[currentPtr].ns != "http://www.w3.org/1999/xhtml") {
                                         pop();
                                     }
                                 }
@@ -3650,11 +3660,9 @@ public final void endTag(ElementName elementName) throws SAXException {
                         case BR:
                             errEndTagBr();
                             if (isInForeign()) {
-                                // XXX can this happen anymore?
                                 errHtmlStartTagInForeignContext(name);
-                                // Check for currentPtr for the fragment
-                                // case.
-                                while (currentPtr >= 0 && stack[currentPtr].ns != "http://www.w3.org/1999/xhtml") {
+                                // Pop foreign elements, but keep context element in fragment case
+                                while (currentPtr > 0 && stack[currentPtr].ns != "http://www.w3.org/1999/xhtml") {
                                     pop();
                                 }
                             }

test-src/nu/validator/htmlparser/test/Html5libTest.java

@@ -102,12 +102,6 @@ private static class TestVisitor extends SimpleFileVisitor<Path> {
 
         private final TestConsumer runner;
 
-        // Files to skip due to known failures unrelated to this parser
-        // (e.g., error reporting differences in foreign content parsing)
-        private static final java.util.Set<String> SKIP_FILES = java.util.Set.of(
-                "foreign-fragment.dat"
-        );
-
         private TestVisitor(boolean skipScripted, String requiredTestExtension,
                 TestConsumer runner) {
             this.skipScripted = skipScripted;
@@ -129,9 +123,7 @@ public FileVisitResult preVisitDirectory(Path dir,
         @Override
         public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
                 throws IOException {
-            String fileName = file.getFileName().toString();
-            if (fileName.endsWith(requiredTestExtension)
-                    && !SKIP_FILES.contains(fileName)) {
+            if (file.getFileName().toString().endsWith(requiredTestExtension)) {
                 runner.accept(file);
             }
             return FileVisitResult.CONTINUE;