Stop using “strictly split” in “validate and extract”

[sideshowbarker]

The “validate and extract” algorithm currently uses “strictly split” to split the qualified name on U+003A (:), which splits on every colon into an array, and takes only the first and second elements from the array. For a qualified name like foo:bar:baz, that produces the prefix foo and the local name bar — silently discarding the :baz part.

This change splits on the first colon only instead, so the local name is bar:baz.

This came up in WICG/sanitizer-api#373. Fixes #1453.

• At least two implementers are interested (and none opposed):

  • WebKit ✅
  • Gecko
  • Chromium

• Tests are written and can be reviewed and commented upon at:

  • WebKit export of https://bugs.webkit.org/show_bug.cgi?id=241419 web-platform-tests/wpt#58331

• Implementation bugs are filed:

  Engine	Bug
  Chromium	https://issues.chromium.org/issues/490251709
  Gecko	https://bugzilla.mozilla.org/show_bug.cgi?id=2021558
  WebKit	[already implemented in https://github.com/WebKit/WebKit/pull/56000]

• MDN issue is filed: N/A

• The top of this comment includes a clear commit message to use.

[josepharhar]

Looks good to me, hopefully there are no compat issues we run into. @otherdaniel

[annevk]

@josepharhar are we all going to try and deploy this and report back if it breaks? @smaug---- does that work for you? If so, we might not need agenda+.

[sideshowbarker]

For the record here, what “validate and extract” said before the patch associated with #1134 was this:

If qualifiedName contains a U+003A (:), then strictly split the string on it and set prefix to the part before and localName to the part after.

So, it was ambiguous — as #1134 pointed out. But, one of the two ways it could have been read as intending is:

If qualifiedName contains a U+003A (:), then strictly split the string on it and set prefix to the part before and localName to the part after.

In other words, it could have been read as actually intending what the change in this PR does. In other-other words, the change here is just reverting the spec back to what it had actually originally been intended to specify.

And nobody in the #1134 discussion ever explicitly instead said anything like, “Throw away everything else in the name after splitting it into a list and taking just the first two list items is clearly what we had meant to require here.”

Instead, that’s just… effectively what it had ended up getting changed to state — without anybody ever having explicitly said there was some good reason for it needing to require throwing away everything else in the name.

[josepharhar]

I thought that using strictly split meant to throw everything after the second colon away because of this discussion: #1387 (comment)

[sideshowbarker]

I thought that using strictly split meant to throw everything after the second colon away because of this discussion: #1387 (comment)

Dunno — but the PR which resulted in the local-name-computation part of the spec ending up as it is now, #1135, is from 2022. So it predates that #1387 (comment) discussion (~8 months ago) by more than 2 years. The only actual spec change made in response to that #1387 (comment) discussion was https://github.com/whatwg/dom/pull/1388/changes — which was, “If prefix is not a valid namespace prefix, then throw” (no new effect on the requirements about the local name).

[smaug----]

@smaug---- does that work for you? If so, we might not need agenda+.

Sounds good to me