Bug 263655
| Summary: | lang attribute in no namespace should be honored only on HTML and SVG elements | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | L. David Baron <dbaron> |
| Component: | DOM | Assignee: | sideshowbarker <mike> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | annevk, ap, cdumez, karlcow, mike, webkit-bug-importer, ysuzuki |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: |
https://github.com/web-platform-tests/wpt/pull/42876 https://github.com/web-platform-tests/wpt/pull/43275 |
||
L. David Baron
As specified in:
https://html.spec.whatwg.org/multipage/dom.html#the-lang-and-xml:lang-attributes
https://www.w3.org/TR/SVG2/struct.html#LangSpaceAttrs
https://github.com/whatwg/html/pull/9796
https://github.com/whatwg/html/pull/9882
the lang attribute in *no* namespace should be honored on HTML and SVG elements, but not on other elements.
Gecko has done this (plus for XUL elements) for a long time, and I'm fixing Chromium to do this in https://crbug.com/1490711 and https://crrev.com/c/4974775.
WebKit, on the other hand, fails the tests for this behavior that I'm adding in https://github.com/web-platform-tests/wpt/pull/42732 , but (at least assuming this change ships successfully in Chromium) it should pass them.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
sideshowbarker
Pull request: https://github.com/WebKit/WebKit/pull/19567
sideshowbarker
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/42876
L. David Baron
For what it's worth, the *other* Chromium change in https://crrev.com/c/4974775 (not the one I filed this bug about) was reverted in https://crrev.com/c/4984312
Radar WebKit Bug Importer
<rdar://problem/117795695>
sideshowbarker
(In reply to L. David Baron from comment #3)
> For what it's worth, the *other* Chromium change in
> https://crrev.com/c/4974775 (not the one I filed this bug about) was
> reverted in https://crrev.com/c/4984312
OK, for the record here (and please correct me if I’m wrong): The lang-related part of that Chromium change has a related spec change at https://github.com/whatwg/html/pull/9880/commits/a45814c7689a86d8e75f05f61619181786092666 — which is to remove from the “To determine the language of a node” algorithm the step at https://html.spec.whatwg.org/multipage/dom.html#language which states this:
> If the node is a slot element whose root is a shadow root: Use the language of that shadow root's host.
…and to also has a related WPT test change at https://github.com/web-platform-tests/wpt/pull/42811/files#diff-3e68420ba9732782c4a09ec7119cd7556e9d1aedef5f2bc98f8d7ef92322a4ab which updated/reverted the related test expectation.
If so, Safari Tech Preview 182 and Safari 17.2 already pass the current test at http://wpt.live/html/dom/elements/global-attributes/lang-attribute-shadow.window.html (which includes the WPT PR 42811 update/revert) — so I think that means WebKit needs no new updates for the lang-related part of the spec changes at least (I’ve not personally looked at all at any of the dir-related spec changes or WPT changes).
sideshowbarker
Incidentally, I had yesterday written a patch that implemented that other lang-related part of the change that’s now been reverted — so I’ll drop the diff in here (because I don’t have any other good place to put it…)
diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
index 731d04aa0198..a549da4299b9 100644
--- a/Source/WebCore/dom/Element.cpp
+++ b/Source/WebCore/dom/Element.cpp
@@ -2578,6 +2578,12 @@ void Element::updateEffectiveLangStateFromParent()
ASSERT(parentNode() != &document());
RefPtr parent = parentOrShadowHostElement();
+ RefPtr shadowRoot = containingShadowRoot();
+
+ if ((parent && parent->isShadowRoot()) || (is<HTMLSlotElement>(*this) && shadowRoot)) {
+ ensureElementRareData().setEffectiveLang(shadowRoot->host()->elementRareData()->effectiveLang());
+ return;
+ }
if (!parent || parent == document().documentElement()) {
setEffectiveLangKnownToMatchDocumentElement(parent.get());
I think I probably could have ended up further reducing that code change, by moving the (is<HTMLSlotElement>(*this) && shadowRoot) check into some of the existing code, rather than as a separate new conditional block — but anyway, it’s all moot now, given that the spec change was reverted.
sideshowbarker
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/43275
EWS
Committed 273726@main (6599021230e5): <https://commits.webkit.org/273726@main>
Reviewed commits have been landed. Closing PR #19567 and removing active labels.