[webkit-reviews] review granted: [Bug 67586] Map 'lang' and xml:lang attributes to '-webkit-locale' CSS property for use with font fallback and text-transform : [Attachment 120000] incorporated review comments

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted  review:
Bug 67586: Map 'lang' and xml:lang attributes to '-webkit-locale' CSS property
for use with font fallback and text-transform
https://bugs.webkit.org/show_bug.cgi?id=67586

Attachment 120000: incorporated review comments
https://bugs.webkit.org/attachment.cgi?id=120000&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=120000&action=review


>>>> Source/WebCore/html/HTMLElement.cpp:211
>>>> +	  } else if (attr->name().matches(XMLNames::langAttr)) {
>>> 
>>> Which of the test cases fails if you change this from matches to ==? I need
to know that to be sure this code is correct.
>> 
>> In lang-mapped-to-webkit-locale.xhtml, all of the test cases with an
"xml:lang" (and without a "lang" with the same value) fail. For example:
>> 
>> <div xml:lang="fr" id="n1">
>> FAIL getLangOfNode('n1') should be fr. Was auto.
>> 
>> Full list of failures:
>> FAIL getLangOfNode('x1') should be ja. Was auto.
>> FAIL getLangOfNode('x2') should be ja. Was auto.
>> FAIL getLangOfNode('x3') should be ja. Was auto.
>> FAIL getLangOfNode('n1') should be fr. Was auto.
>> FAIL getLangOfNode('n2') should be fr. Was auto.
>> FAIL getLangOfNode('p1') should be ja. Was auto.
>> FAIL getLangOfNode('q3') should be auto. Was ja.
>> FAIL getLangOfNode('q4') should be ar. Was ja.
>> FAIL getLangOfNode('q5') should be auto. Was ja.
> 
> The use of matches here instead of == is indeed wrong or at least unneeded,
and since == is more efficient we should not use matches.
> 
> The use of matches is a workaround for bug 75066, which should be fixed
instead.

I was wrong again. The use of matches is correct. We could use "==" because of
the special rules about the xml prefix, but we don’t need to do that
optimization.