Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1180)

Issue 2533033003: Fix the uppercasing of U+00E7(ç) and U+00F7(÷) (Closed)

Created:
4 years ago by jungshik at Google
Modified:
4 years ago
Reviewers:
Dan Ehrenberg
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Fix the uppercasing of U+00E7(ç) and U+00F7(÷) Due to a typo in runtime-i18n.js, 'ç'(U+00E7) was not uppercased while '÷'(U+00F7) was incorrectly uppercased to '×'(U+00D7). Add a comprehensive test for Latin-1 supplemental block (U+00A0 ~ U+00FF). (they're special-cased for speed-up and needs to have a test for the range.). TEST=intl/general/case-mapping BUG=v8:5681 Committed: https://crrev.com/2f5da9a551785899ae1c899fd3c31b834a731e7d Cr-Commit-Position: refs/heads/master@{#41331}

Patch Set 1 #

Patch Set 2 : update the comment #

Patch Set 3 : Use FastAsciiConvert for uppercasing #

Patch Set 4 : use template for FastAsciiConvert #

Patch Set 5 : use FastAsciiConvert for lowercase as well #

Patch Set 6 : a bit more tweaks + tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+203 lines, -149 lines) Patch
M BUILD.gn View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime/runtime-i18n.cc View 1 2 3 4 5 5 chunks +39 lines, -31 lines 0 comments Download
M src/runtime/runtime-strings.cc View 1 2 3 2 chunks +1 line, -117 lines 0 comments Download
M src/runtime/runtime-utils.h View 1 2 3 2 chunks +31 lines, -0 lines 0 comments Download
A src/runtime/runtime-utils.cc View 1 2 3 1 chunk +110 lines, -0 lines 0 comments Download
M src/v8.gyp View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M test/intl/general/case-mapping.js View 1 2 3 4 5 1 chunk +20 lines, -1 line 0 comments Download

Messages

Total messages: 12 (5 generated)
jungshik at Google
Dan, can you take a look? Thanks
4 years ago (2016-11-28 20:41:56 UTC) #2
Dan Ehrenberg
On 2016/11/28 20:41:56, jungshik at google wrote: > Dan, can you take a look? Thanks ...
4 years ago (2016-11-28 22:07:33 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2533033003/20001
4 years ago (2016-11-28 22:28:08 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-11-28 22:55:29 UTC) #8
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/2f5da9a551785899ae1c899fd3c31b834a731e7d Cr-Commit-Position: refs/heads/master@{#41331}
4 years ago (2016-11-28 22:55:57 UTC) #10
jungshik at Google
On 2016/11/28 22:55:57, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as ...
4 years ago (2016-11-30 17:44:28 UTC) #11
jungshik at Google
4 years ago (2016-11-30 17:51:50 UTC) #12
Message was sent while issue was closed.
On 2016/11/30 17:44:28, jungshik at google wrote:
> On 2016/11/28 22:55:57, commit-bot: I haz the power wrote:
> > Patchset 2 (id:??) landed as
> > https://crrev.com/2f5da9a551785899ae1c899fd3c31b834a731e7d
> > Cr-Commit-Position: refs/heads/master@{#41331}
> 
> ooops. I messed up. PS #3 ~ PS #6 are NOT for this CL.

https://codereview.chromium.org/2533983006/ is a new CL for PS #3 ~ PS #6.

Powered by Google App Engine
This is Rietveld 408576698