New issue
Advanced search Search tips

Issue 613331 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Aug 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 613358

Blocking:
issue 475351



Sign in to add a comment

Fix more plural messages and 'native' digit issues along the way if applicable

Project Member Reported by js...@chromium.org, May 19 2016

Issue description

Spun off from bug 475351

Searching for '<ex>[0-9]</ex>' found some more cases where plural handling is necessary and/or native digits have to be used. 

https://code.google.com/p/chromium/codesearch#search/&q=file:grd%20%3Cex%3E%5B0-9%5D%3C%5C/ex%3E&sq=package:chromium&type=cs

'<ex>[0-9]+</ex> has 6 more. Not all of them are relevant, though.

https://code.google.com/p/chromium/codesearch#search/&q=file:grd%20%3Cex%3E%5B0-9%5D%2B%3C%5C/ex%3E&sq=package:chromium&type=cs


Besides, searching for GetStringFUTF16Int turned up a few more. 

(U)Int*ToString* may turn up a few more. 

 

Comment 1 by js...@chromium.org, May 19 2016

Summary: Fix more plural messages (and 'native' digit issues if applicable) (was: Fix more plural messages as well as 'native' digit issues along the way)

Comment 2 by js...@chromium.org, May 19 2016

As for digits (native or not), there are interesting cases (serial number in hexadecimal,  passkey, passcode in decimal)


void BluetoothNotificationController::DisplayPasskey(BluetoothDevice* device,
                                                     uint32_t passkey) {
  base::string16 message = l10n_util::GetStringFUTF16(
          IDS_ASH_STATUS_TRAY_BLUETOOTH_DISPLAY_PASSKEY,
          device->GetName(), base::UTF8ToUTF16(
              base::StringPrintf("%06i", passkey)));

  NotifyPairing(device, message, false);
}

There are tons of StringPrintf with various format specifier. The vast majority of them are not used in the UI, though, but some of them might.

Comment 3 by js...@chromium.org, May 20 2016

Cc: glevin@chromium.org
Summary: Fix more plural messages and 'native' digit issues along the way if applicable (was: Fix more plural messages (and 'native' digit issues if applicable))
My number in comment 0 is a number of grd files with this issue ( < 20). The actual number of messages with '<ex>[0-9]+</ex>' is over 90 !  
Reviewing them all and fixing them take rather long (see bug 613347)

Comment 4 by js...@chromium.org, May 21 2016

https://codereview.chromium.org/2002783002 : a first try (a partial fix).
 
There are more strings to review and fix if necessary. 

I left alone 'percent' because it's being taken care of by Greg elsewhere. 

Comment 5 by js...@chromium.org, May 31 2016

Blockedon: 613358
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bccecf36c6ae6bf612174eaaac7effec854877ee

commit bccecf36c6ae6bf612174eaaac7effec854877ee
Author: jshin <jshin@chromium.org>
Date: Fri Jun 03 02:59:58 2016

Use plural formats and native digits

With codesearch, look for '<ex>[0-9]+</ex> file:grd'. Review each of
matches and change them to use plural formats(C++), toLocaleString(JavaScript),
or other i18n-safe ways.

Moreover, change GetStringFUTF16Int() to use 'FormatNumber' instead
of Int*ToString(). Call sites of GetStringFUTF16Int() were reviewed
Where native digits are not appropriate (non-UI), call sites were
adjusted.

Besides, add a warning about 'ASCII digits vs native digits' to
base/strings/string_number_conversions.h and add more details
on GetPlural* to ui/base/l10n/l10n_util.h.

BUG= 613331 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation;tryserver.blink:linux_blink_rel,mac_blink_rel

Review-Url: https://codereview.chromium.org/2002783002
Cr-Commit-Position: refs/heads/master@{#397608}

[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/ash/system/chromeos/network/tray_sms.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/base/strings/string_number_conversions.h
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/app/bookmarks_strings.grdp
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/app/generated_resources.grd
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/media/desktop_media_list_ash.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/media/native_desktop_media_list.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/profiles/profile_attributes_storage.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/resources/history/history.js
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/resources/history/other_devices.js
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/resources/print_preview/settings/dpi_settings.js
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/components/crash/core/browser/resources/crashes.js
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/ios/chrome/app/resources/history/other_devices.js
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/third_party/WebKit/Source/core/layout/LayoutTheme.cpp
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/third_party/WebKit/Source/core/layout/LayoutThemeMac.mm
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/ui/base/l10n/l10n_util.cc
[modify] https://crrev.com/bccecf36c6ae6bf612174eaaac7effec854877ee/ui/base/l10n/l10n_util.h

Comment 7 by js...@chromium.org, Jul 25 2016

bug 617540 fixed one more (in JS) with toLocaleString(). 
Status: Archived (was: Started)
Archiving old bugs that haven't been modified in over two years. 

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!

Sign in to add a comment