Bug 263655 - lang attribute in no namespace should be honored only on HTML and SVG elements
Summary: lang attribute in no namespace should be honored only on HTML and SVG elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: sideshowbarker
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2023-10-25 07:32 PDT by L. David Baron
Modified: 2024-01-29 21:27 PST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description L. David Baron 2023-10-25 07:32:05 PDT
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.
Comment 1 sideshowbarker 2023-10-25 14:52:59 PDT
Pull request: https://github.com/WebKit/WebKit/pull/19567
Comment 2 sideshowbarker 2023-10-31 16:13:21 PDT
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/42876
Comment 3 L. David Baron 2023-11-01 05:53:23 PDT
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
Comment 4 Radar WebKit Bug Importer 2023-11-01 07:33:15 PDT
<rdar://problem/117795695>
Comment 5 sideshowbarker 2023-11-01 22:34:59 PDT
(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).
Comment 6 sideshowbarker 2023-11-01 23:02:57 PDT
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.
Comment 7 sideshowbarker 2023-11-20 19:06:05 PST
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/43275
Comment 8 EWS 2024-01-29 21:27:06 PST
Committed 273726@main (6599021230e5): <https://commits.webkit.org/273726@main>

Reviewed commits have been landed. Closing PR #19567 and removing active labels.