Handle unmatched closing tags and SVG parsing

[olavoasantos]

Problem

The parser popped one frame for every closing tag without checking its name. An unmatched closing tag could therefore close the current element and redirect following content into the wrong parent. It also created all parsed elements with the HTML factory, so SVG markup assigned through innerHTML lost SVG namespace and element semantics.

Impact

Minor. The exposed limited parser could build a different tree after an unmatched closing tag, and parsed SVG could be an HTML Element rather than an SVGElement. That affects subsequent tree queries, namespace-sensitive operations, and serialized worker-side DOM output.

Reproduction

const element = document.createElement('div');
element.innerHTML =
  '<section><span>Before</wrong><b>After</b></span></section>';

Before this change, </wrong> popped the span frame, making b a child of section. It now ignores the unmatched closing tag and serializes as <section><span>Before<b>After</b></span></section>. Parsed <svg> descendants now retain the SVG namespace rather than being created as HTML elements.

Change

• Match closing tags against the open-element stack, ignoring unmatched tags and closing intervening elements only when an ancestor tag matches.
• Create <svg> and its descendants in the SVG namespace; inherit an SVG innerHTML context; and switch to HTML inside foreignObject, with nested <svg> switching back to SVG.
• Preserve SVG names and keep SVG <template> elements as SVG elements rather than HTML templates.

The change deliberately improves these supported parser paths without claiming full browser-grade malformed-markup recovery or foreign-content parsing.

Tests

Extends packages/polyfill/source/tests/inner-html-parsing.test.ts with unmatched and ancestor closing-tag cases, top-level and inherited SVG namespace assertions, foreignObject and nested-SVG boundaries, template insertion-target preservation after an unmatched closing tag, and SVG template-descendant behavior.

Stack

• Stack: HTML serialization (#701), layer 3 of 3
• Base: serialize-template-content
• Review after PR Parse and serialize template content in HTML output #686.

The nullable-namespace type correction now belongs to #686, leaving this layer scoped to closing-tag and SVG parsing behavior.

Validation

Fresh GitHub CI on restacked head 2891a92 passes:

• lint;
• type-check and build;
• unit tests with coverage;
• bundle-size checks;
• Playwright end-to-end tests;
• the classified Web Platform Test suite.

[JoviDeCroock]

I think the following is still problematic

   <section>
     <template>
       Before
       </section>
       After
     </template>

not sure what the expected result should be though, we search backwards through every open frame which means we forget that we should be closing template

[henrytao-me]

Thanks for the template-boundary fix and regression test in the restack. That addresses the earlier scope concern. Two corrections remain in the SVG parsing branch at 81f87c89.

1. Preserve SVG-to-HTML re-entry

The namespace selection only recognizes a lowercased, unprefixed foreignObject name:

const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
const title = document.createElementNS(SVG_NAMESPACE, 'title');
title.innerHTML = '<x-label>Help</x-label>';

Native/base create an HTML child; the reviewed implementation creates an SVG child. With a registered x-label, its constructor runs once on native/base and zero times on the reviewed implementation. SVG desc and prefixed s:foreignObject contexts have the same regression. Unprefixed foreignObject works.

HTML integration points depend on namespace plus exact localName, not lowercased qualified name. Please include title/desc and prefixed foreignObject, while keeping programmatically created SVG elements with established localNames foreignobject or ForeignObject out of that integration-point set.

There is also a distinct HTML breakout path: assigning <p>content</p> to SVG innerHTML produces an HTML paragraph on native/base, but an SVG paragraph here. Other HTML breakout tags and conditional font color/face/size cases reproduce this. Correcting the integration-point predicate alone does not fix those paths; preserve the HTML re-entry rules and parent placement without treating every unknown SVG tag as HTML.

2. Don't interpret parsed HTML token names as XML QNames

The new document.createElementNS(namespace, tag) call changes accepted literal-colon token names:

const svg = document.createElementNS(SVG_NAMESPACE, 'svg');
svg.innerHTML = '<xml:item></xml:item>';
// Reviewed implementation: NamespaceError
// Native: accepted; localName === 'xml:item', prefix === null

<p:shape></p:shape> similarly becomes localName shape / prefix p, rather than native's literal localName p:shape / null prefix. Base accepted these inputs but assigned the wrong namespace; the new rejection/name splitting is separate from that inherited namespace defect.

Please use the existing internal Document.ts:createElement(ownerDocument, name, namespace) primitive for parsed local names. It preserves the selected namespace, null prefix, ownership and hooks without reparsing the token as a QName. Keep the public createElementNS() validation unchanged.

Focused coverage: integration-point namespaces and constructor counts; non-integration controls; HTML breakout namespaces/placement; literal-colon localName/prefix/namespace and no parser exception, with public QName-validation controls still rejecting invalid API arguments. These are parser-specific corrections, not a request to rewrite the whole HTML parser.

Evidence boundary: the native/base/head repros and full focused suites were run at 2891a92 against 27f275e. I rechecked the source at 81f87c89: these two branches are unchanged, while template scope is fixed. I have not run a complete new-head/base validation after the restacks. No approval or live-stack sign-off implied.

[olavoasantos]

i checked this case against native parsing. The expected result is <section><template>BeforeAfter</template></section>. The backward end-tag search now stops at the nearest HTML template frame unless we're closing that template, and the exact case is covered.

[andrewiggins]

Some of Henry's comment isn't yet addressed. Let us know when those are addressed and I'll come back and review.

Although, I'd consider deferring supporting HTML breakout tags in SVG initially. That seems like a slightly larger change then just "adding SVG parsing" that I'd like to weigh separately to see how much it costs.

[andrewiggins]

I don't think we need to address this right now, but just FYI - this is a quadratic look up in a catastrophic case of malformed markup. I think that's fine for now but just want to note it for posterity if we come across this being a problem in the future.

[olavoasantos]

yea, agreed. let's keep the quadratic malformed-input case as a known tradeoff for now instead of expanding this PR.

[olavoasantos]

The remaining SVG cases are addressed on e164e79:

• HTML integration points now use SVG namespace plus exact localName for foreignObject, title, and desc, including prefixed foreignObject; established foreignobject / ForeignObject names stay SVG.
• tokenizer-produced xml:item and p:shape names now go through the internal CREATE_ELEMENT path, preserving literal local names and null prefixes without weakening public createElementNS() validation.
• breakout handling is limited to the specified token set and conditional font attribute names, including SVG-frame placement. Values like title=" color " and names like data-color do not trigger it.

@andrewiggins leaving breakout out would keep the regression against base where <p> changed from HTML to SVG. i kept the correction bounded to namespace routing and frame placement — no insertion-mode engine, adoption agency, MathML, SVG adjustment tables, or broader foreign-content parser. Should we keep that bounded correction here, or would you still prefer splitting it? Fresh CI is green on the rewritten head.

[henrytao-me]

Thanks, the SVG integration-point, breakout, and literal-colon/QName corrections now pass on e164e79a against 0a393898. The template-boundary fix also checks out.

One remaining P2 correction: please keep the namespace routing SVG-scoped, or explicitly preserve the already-working non-SVG HTML integration contexts. At serialization.ts:119, let namespace = openElement[NS] inherits every foreign namespace, not only SVG. This changes existing behavior outside the stated scope.

Here is a minimal regression test using only public APIs:

it('preserves HTML custom elements in an existing non-SVG context', () => {
  const window = new Window();
  const document = window.document;
  let constructions = 0;

  class Label extends window.HTMLElement {
    constructor() {
      super();
      constructions += 1;
    }
  }
  window.customElements.define('x-label', Label);

  const context = document.createElementNS(
    'http://www.w3.org/1998/Math/MathML',
    'mtext',
  );
  context.innerHTML = '<x-label>Help</x-label>';

  expect(context.children[0].namespaceURI).toBe(
    'http://www.w3.org/1999/xhtml',
  );
  expect(context.children[0]).toBeInstanceOf(Label);
  expect(constructions).toBe(1);
});

Native and actual base create an HTML child and invoke the registered constructor once. This head creates a generic MathML element and invokes it zero times. The same issue reproduces for mi, mo, mn, ms, prefixed mtext, and annotation-xml with HTML encodings. Six standalone regression tests pass on base and fail on head; native comparisons confirm the corresponding behavior.

This also has observable core impact: with a registered RemoteElement, the wrong namespace bypasses its constructor and attribute handling. A real RemoteReceiver misses the initial attribute and later attribute updates; text still synchronizes. Base instantiates the class and synchronizes both.

The suggestion is to preserve existing behavior while adding SVG support, not to implement a full MathML parser. Please retain the now-passing SVG integration/non-integration controls, breakout placement, and public QName validation while adding the non-SVG regression coverage. This broad inheritance was present in the earlier implementation too; it is not a new restack-only defect.

Validation: 393 existing polyfill tests, 63 focused core tests, locked polyfill typecheck, formatting, and current CI pass. Four actual-core SVG flows pass; the non-SVG flow above regresses. No production fix or full locked-repository/live Shell/admin validation performed. The separate maintainer discussion about breakout scope remains open.