[webkit-reviews] review granted: [Bug 97929] WebKit Doesn't Recognize Content-Language HTTP Header : [Attachment 168973] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Brian White
<bcwhite at chromium.org>'s request for review:
Bug 97929: WebKit Doesn't Recognize Content-Language HTTP Header
https://bugs.webkit.org/show_bug.cgi?id=97929

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

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


> Source/WebCore/loader/FrameLoader.cpp:673
> +	       headerContentLanguage.truncate(commaIndex); // notFound == -1 ==
don't truncate

This comment is somewhat misleading - notFound is static_cast<size_t>(-1), not
-1. I'd either omit the comment, or say something like "won't truncate if comma
was not found".

> Source/WebCore/loader/FrameLoader.cpp:674
> +	       headerContentLanguage =
headerContentLanguage.stripWhiteSpace(isHTMLSpace);

Haven't noticed this before. Why HTML space? We're not dealing with HTML here.

> LayoutTests/http/tests/misc/extract-http-content-language.php:7
> +<script src="../../js-test-resources/js-test-pre.js"></script>

A trivial nit - no need for relative path in HTTP tests. I think that
"/js-test-resources/js-test-pre.js" would work just as well.