TextCodecICU::encode is replacing '\' by '¥'. But it is also doing so for the characters afer '\'. The string "\cba" will be encoded as '¥¥¥¥'.
Created attachment 232787 [details] Patch
Comment on attachment 232787 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232787&action=review > LayoutTests/fast/encoding/backslash-encoding-jp.html:3 > + <title>Bug 133678: TextCodecICU::encode should not change characters other characters than backslash</title> The title doesn't parse. I'd say "TextCodecICU::encode turns the whole string into yen signs if there are any backslashes in it". In a bug title, it's better to describe the symptom, not what the code should do. Also, it's better to have a <p> element than a <title>, because this way, the output contains the description of what is being tested, not only a "PASS". > LayoutTests/fast/encoding/backslash-encoding-jp.html:12 > + var toBeEncoded = "\\abcde"; > + var expectedEncoded = "%5Cabcde"; This test only checks that characters after the backslash are correct, but the patch also fixes characters before the backslash, as a separate code change. I.e. please test "ab\\cde", not "\\abcde".
Created attachment 233137 [details] Updated test and title
(In reply to comment #2) > (From update of attachment 232787 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232787&action=review > > > LayoutTests/fast/encoding/backslash-encoding-jp.html:3 > > + <title>Bug 133678: TextCodecICU::encode should not change characters other characters than backslash</title> > > The title doesn't parse. I'd say "TextCodecICU::encode turns the whole string into yen signs if there are any backslashes in it". In a bug title, it's better to describe the symptom, not what the code should do. > > Also, it's better to have a <p> element than a <title>, because this way, the output contains the description of what is being tested, not only a "PASS". > > > LayoutTests/fast/encoding/backslash-encoding-jp.html:12 > > + var toBeEncoded = "\\abcde"; > > + var expectedEncoded = "%5Cabcde"; > > This test only checks that characters after the backslash are correct, but the patch also fixes characters before the backslash, as a separate code change. I.e. please test "ab\\cde", not "\\abcde". Thanks for the review. The uploaded patch is updated according your suggestions.
Comment on attachment 233137 [details] Updated test and title Attachment 233137 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5250254710505472 New failing tests: media/W3C/video/networkState/networkState_during_loadstart.html
Created attachment 233138 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 233137 [details] Updated test and title Clearing flags on attachment: 233137 Committed r170573: <http://trac.webkit.org/changeset/170573>
All reviewed patches have been landed. Closing bug.