-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The letter 'ç' is no longer upper-cased by .toUpperCase from 7.1.0 #9785
Comments
Does it work when you install the full-icu package? V8 delegates more to ICU in v7.x and the release binaries only include en_US because of size constraints. |
Hello @bnoordhuis, I tried the following: git clone [email protected]:Yomguithereal/talisman.git
cd talisman
git checkout 003d44a3aaa81155331aaa23bc7708fa489428d6
npm install
npm install full-icu
npm test I still have the same problem. Did I do it wrong? |
I could replicate this bug on Linux Mint : $uname -a
Linux ### 3.19.0-32-generic #37~14.04.1-Ubuntu SMP Thu Oct 22 09:41:40 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux |
cc @nodejs/intl |
I tried to build nodejs up to the 0f871e1 commit and uppercasing works as expected. This seems to confirm that the introduction of |
confirmed here too
|
It's not that commit - it's ICU 58.1. Applying ICU 57 on top of current HEAD also gives the right result. Or rather, some interaction between node/v8 and ICU 58. Because ICU's own |
Also |
The latest v8 (ToT) does not use el-Upper transliterator any more and just uses ICU's uppercase API. I'll test with ToT v8. |
Seems to be something wrong in v8's optimizations under a couple possible workarounds
|
I can reproduce the bug with the latest ToT (without reading the bug much, I thought it's about Greek) when --icu_case_mapping flag is on. So, there's obviously a bug. Thank you for the report. |
@jungshik It's Bug synergy!! |
There was a typo in ToLatin1Upper() ( E7 should be F7) in runtime-i18n.cc |
It's fixed in v8 ToT: https://bugs.chromium.org/p/v8/issues/detail?id=5681&desc=2 |
Original commit message: 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 Review-Url: https://codereview.chromium.org/2533033003 Cr-Commit-Position: refs/heads/master@{nodejs#41331} PR-URL: nodejs#9828 Fixes: nodejs#9785
* add test for ç/Ç in various locales Reviewed-by: TBD PR-URL: nodejs#9828 Fixes: nodejs#9785
* add test for ç/Ç in various locales PR-URL: #9828 Fixes: #9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
* add test for ç/Ç in various locales PR-URL: nodejs#9828 Fixes: nodejs#9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
* add test for ç/Ç in various locales PR-URL: nodejs#9828 Fixes: nodejs#9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Original commit message: 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 Review-Url: https://codereview.chromium.org/2533033003 Cr-Commit-Position: refs/heads/master@{#41331} PR-URL: #9828 Fixes: #9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
* add test for ç/Ç in various locales PR-URL: #9828 Fixes: #9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Original commit message: 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 Review-Url: https://codereview.chromium.org/2533033003 Cr-Commit-Position: refs/heads/master@{nodejs#41331} PR-URL: nodejs#9828 Fixes: nodejs#9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
* add test for ç/Ç in various locales PR-URL: nodejs#9828 Fixes: nodejs#9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Original commit message: 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 Review-Url: https://codereview.chromium.org/2533033003 Cr-Commit-Position: refs/heads/master@{nodejs#41331} PR-URL: nodejs#9828 Fixes: nodejs#9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
* add test for ç/Ç in various locales PR-URL: nodejs#9828 Fixes: nodejs#9785 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Hello @nodejs,
I stumbled upon what seems to be a bug in node.js v7.1.0.
In node v6.9.1 & v7.0.0, we have the following:
while in node v7.1.0
I noticed this because it made a CI build fail (ref) and broke some of the phonetic algorithms of the Talisman library.
Sorry if this is normal and I did not read the changelogs carefully enough.
Have a good day.
The text was updated successfully, but these errors were encountered: