Bug 133678

Summary:

TextCodecICU::encode turns the whole string as yen signs if there is any backslash in it

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

Text

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, buildbot, commit-queue, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

109199

Attachments:

Description	Flags
Patch	none
Updated test and title	none
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2	none

youenn fablet

Reported 2014-06-10 08:20:10 PDT

TextCodecICU::encode is replacing '\' by '¥'. But it is also doing so for the characters afer '\'. The string "\cba" will be encoded as '¥¥¥¥'.

Attachments
Patch (4.75 KB, patch) 2014-06-10 08:40 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Updated test and title (4.95 KB, patch) 2014-06-15 03:25 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (522.27 KB, application/zip) 2014-06-15 06:40 PDT, Build Bot	no flags	Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2014-06-10 08:40:19 PDT

Created attachment 232787 [details] Patch

Alexey Proskuryakov

Comment 2 2014-06-14 14:41:54 PDT

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".

youenn fablet

Comment 3 2014-06-15 03:25:04 PDT

Created attachment 233137 [details] Updated test and title

youenn fablet

Comment 4 2014-06-15 04:41:44 PDT

(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.

Build Bot

Comment 5 2014-06-15 06:40:34 PDT

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

Build Bot

Comment 6 2014-06-15 06:40:36 PDT

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

WebKit Commit Bot

Comment 7 2014-06-29 10:10:53 PDT

Comment on attachment 233137 [details] Updated test and title Clearing flags on attachment: 233137 Committed r170573: <http://trac.webkit.org/changeset/170573>

WebKit Commit Bot

Comment 8 2014-06-29 10:10:57 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.