Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2498653002:
Return one U+fffd for longest subpart of incomplete utf-8 character. (Closed)
DescriptionReturn one U+fffd for longest subpart of incomplete utf-8 character.
This brings the utf-8 decoder in line w/ the Unicode spec
recommendation, and also with the utf-8 decoder in V8. This
latter bit is the motivation for this change, since either this
or the v8 utf-8 decoder gets used for JavaScript source code, and
they should agree with each other.
More details + spec references in the bug.
BUG=chromium:662822
Committed: https://crrev.com/9a8c03cf61f42e45e4c4d2362d97cc2c5abbdc1f
Cr-Commit-Position: refs/heads/master@{#432470}
Patch Set 1 #Patch Set 2 : Updated test expectations. #
Total comments: 4
Patch Set 3 : Remove dead assignments. #
Total comments: 2
Patch Set 4 : Fix comment. #
Messages
Total messages: 25 (16 generated)
The CQ bit was checked by vogelheim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by vogelheim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
vogelheim@chromium.org changed reviewers: + jochen@chromium.org, marja@chromium.org
Description was changed from ========== Return one U+fffd for longest subpar of incomplete utf-8 character. This brings the utf-8 decoder in line w/ the Unicode spec recommendation, and also with the utf-8 decoder in V8. This latter bit is the motivation for this change, since either this or the v8 utf-8 decoder gets used for JavaScript source code, and they should agree with each other. More details + spec references in the bug. BUG=chromium:662822 ========== to ========== Return one U+fffd for longest subpart of incomplete utf-8 character. This brings the utf-8 decoder in line w/ the Unicode spec recommendation, and also with the utf-8 decoder in V8. This latter bit is the motivation for this change, since either this or the v8 utf-8 decoder gets used for JavaScript source code, and they should agree with each other. More details + spec references in the bug. BUG=chromium:662822 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Posting comments from in-person code review https://codereview.chromium.org/2498653002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp (right): https://codereview.chromium.org/2498653002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp:269: count = -character; Looks like count doesn't affect anything https://codereview.chromium.org/2498653002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp:354: count = -character; Ditto
The CQ bit was checked by vogelheim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2498653002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp (right): https://codereview.chromium.org/2498653002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp:269: count = -character; On 2016/11/16 08:41:38, marja wrote: > Looks like count doesn't affect anything Removed. https://codereview.chromium.org/2498653002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp:354: count = -character; On 2016/11/16 08:41:38, marja wrote: > Ditto Removed. (All actual processing of non-ASCII characters is done in upConvertTo16Bit.)
lgtm % comment https://codereview.chromium.org/2498653002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp (right): https://codereview.chromium.org/2498653002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp:437: // Each error generates a replacement character and consumes one byte. Pls fix this comment
https://codereview.chromium.org/2498653002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp (right): https://codereview.chromium.org/2498653002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/wtf/text/TextCodecUTF8.cpp:437: // Each error generates a replacement character and consumes one byte. On 2016/11/16 09:59:14, marja wrote: > Pls fix this comment Done.
The CQ bit was checked by vogelheim@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org, marja@chromium.org Link to the patchset: https://codereview.chromium.org/2498653002/#ps60001 (title: "Fix comment.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Return one U+fffd for longest subpart of incomplete utf-8 character. This brings the utf-8 decoder in line w/ the Unicode spec recommendation, and also with the utf-8 decoder in V8. This latter bit is the motivation for this change, since either this or the v8 utf-8 decoder gets used for JavaScript source code, and they should agree with each other. More details + spec references in the bug. BUG=chromium:662822 ========== to ========== Return one U+fffd for longest subpart of incomplete utf-8 character. This brings the utf-8 decoder in line w/ the Unicode spec recommendation, and also with the utf-8 decoder in V8. This latter bit is the motivation for this change, since either this or the v8 utf-8 decoder gets used for JavaScript source code, and they should agree with each other. More details + spec references in the bug. BUG=chromium:662822 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Return one U+fffd for longest subpart of incomplete utf-8 character. This brings the utf-8 decoder in line w/ the Unicode spec recommendation, and also with the utf-8 decoder in V8. This latter bit is the motivation for this change, since either this or the v8 utf-8 decoder gets used for JavaScript source code, and they should agree with each other. More details + spec references in the bug. BUG=chromium:662822 ========== to ========== Return one U+fffd for longest subpart of incomplete utf-8 character. This brings the utf-8 decoder in line w/ the Unicode spec recommendation, and also with the utf-8 decoder in V8. This latter bit is the motivation for this change, since either this or the v8 utf-8 decoder gets used for JavaScript source code, and they should agree with each other. More details + spec references in the bug. BUG=chromium:662822 Committed: https://crrev.com/9a8c03cf61f42e45e4c4d2362d97cc2c5abbdc1f Cr-Commit-Position: refs/heads/master@{#432470} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/9a8c03cf61f42e45e4c4d2362d97cc2c5abbdc1f Cr-Commit-Position: refs/heads/master@{#432470} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
