[webkit-reviews] review denied: [Bug 179416] Remove support for iOS-only softbank-sjis encoding if possible : [Attachment 326408] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Maciej Stachowiak
<mjs at apple.com>'s request for review:
Bug 179416: Remove support for iOS-only softbank-sjis encoding if possible
https://bugs.webkit.org/show_bug.cgi?id=179416

Attachment 326408: Patch

https://bugs.webkit.org/attachment.cgi?id=326408&action=review




--- Comment #8 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 326408
  --> https://bugs.webkit.org/attachment.cgi?id=326408
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326408&action=review

This patch only has LayoutTests changes, nothing in WebCore.

> LayoutTests/fast/encoding/charset-softbank-sjis.html:5
> +    <meta charset=utf-8>

Pretty sure that default encoding is well defined in DRT/WKTR, so I'd omit this
line. I feel like we are unnecessarily relying on parser details here.

The small downside is that the test will fail locally. One way to correct for
that is to check that document.charset is not softbank-sjis (or doesn't include
"jis", for some resilience). Or you could print the actual charset and have
text explaining that the test passes as long as it's not softbank-sjis.

> LayoutTests/fast/encoding/charset-softbank-sjis.html:9
> +    if (window.testRunner)
> +	   testRunner.dumpAsText();

js-test-pre.js enables dumpAsText.

I recommend using js-test.js in new tests instead, as is saves one line from
boilerplate.