[webkit-reviews] review granted: [Bug 179416] Remove support for iOS-only softbank-sjis encoding if possible : [Attachment 326431] Patch with test that works, and done the way Alexey suggested

Wed Nov 8 23:51:16 PST 2017

Alexey Proskuryakov <ap at webkit.org> has granted 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 326431: Patch with test that works, and done the way Alexey
suggested

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

--- Comment #24 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 326431
  --> https://bugs.webkit.org/attachment.cgi?id=326431
Patch with test that works, and done the way Alexey suggested

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

> LayoutTests/fast/encoding/charset-softbank-sjis-expected.txt:5
> +TEST COMPLETE
> +Test PASSED if this renders as an accented e: âÃ©â.

This is not right.

> LayoutTests/fast/encoding/resources/softbank-sjis-iframe.html:5
> +<script src="../../../resources/js-test.js"></script>

I think that this should be in the main frame. Otherwise, it will call
notifyDone once the subframe is done loading, which may be before the main
frame is done.

> LayoutTests/fast/encoding/resources/softbank-sjis-iframe.html:11
> +Test PASSED if this renders as an accented e: âÃ©â.

I think that it would work better if this were at the top of <body> (and I'd
make it an explicit element). But also, why not put it into a script as a
shouldBe? I think that this should do it:

shouldBe("'\xe9'", "'é'");