[webkit-reviews] review denied: [Bug 48225] Language attribute validation : [Attachment 71901] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Leandro Graciá Gil
<leandrogracia at chromium.org>'s request for review:
Bug 48225: Language attribute validation
https://bugs.webkit.org/show_bug.cgi?id=48225

Attachment 71901: Patch
https://bugs.webkit.org/attachment.cgi?id=71901&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71901&action=review

> LayoutTests/ChangeLog:9
> +	   The contents of the language attribute are expected to follow the
BCP 47
> +	   specification. However no validation at all is performed on it. This
patch

What are the specs governing this? In HTML5, I could find that in two places:
- authoring requirements, which are irrelevant to browsers;
- non-normative list of attributes, saying that value of hreflang, lang, and
srclang attributes is a "valid BCP 47 language tag".

So, I don't see that requirement specified anywhere. And Firefox 3.6 fails the
test case included in this patch. Does IE pass it?

> LayoutTests/fast/dom/Element/language-valid.html:9
> +	   p:lang(en) { text-transform: uppercase; }
> +	   p:lang(iην41iδ_セ4g) { text-transform: uppercase; }

We'd need tests for more forbidden characters - ideally, for every forbidden
ASCII character, like en/GB.

Please use red/green color in results for easier visual evaluation
<http://www.w3.org/Style/CSS/Test/guidelines.html#color>.

> WebCore/dom/Element.cpp:1513
> +	       if (!isASCIIAlphanumeric(c) && c != '-')
> +		   return AtomicString();

Is an invalid value equivalent to lang="", or should it be ignored?