Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] Ordered vs unordered placeables in strings #7212

Closed
flodolo opened this issue Dec 16, 2019 · 4 comments
Closed

[Bug] Ordered vs unordered placeables in strings #7212

flodolo opened this issue Dec 16, 2019 · 4 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:health Improve code health 🌐 L10N Localization, translation, strings, .. P1 Current sprint

Comments

@flodolo
Copy link

flodolo commented Dec 16, 2019

In Firefox desktop it's possible to switch from unordered to ordered placeables in the localized strings (and the other way around). For exampleClick %S to open %S is equivalent to Click %1$S to open %2$S.

That's handy, and also needed in cases where the locale needs to change the order of parameters compared to en-US.

Now, it looks like some locales are doing the same thing in Fenix. Example, this string

    <!-- Text for displaying the default device name.
        The first parameter is the application name, the second is the device manufacturer name
        and the third is the device model. -->
    <string name="default_device_name">%s on %s %s</string>

In Japanese becomes:

    <string name="default_device_name">%2$s %3$s 上の %1$s</string>

Can someone verify in product that this is working? In general, it would be great to always use ordered placeables in the source string when there's more than one, because not every language is aware that you can swap them (assuming that actually works in Android).

cc @Delphine @Pike

┆Issue is synchronized with this Jira Task

@flodolo flodolo added the 🐞 bug Crashes, Something isn't working, .. label Dec 16, 2019
@boek
Copy link
Contributor

boek commented Dec 19, 2019

@flodolo You're right, we should definitely switch to this. We can start doing this for all new strings immediately. How should we migrate the existing strings?

@boek boek added eng:health Improve code health 🌐 L10N Localization, translation, strings, .. labels Dec 19, 2019
@flodolo
Copy link
Author

flodolo commented Dec 19, 2019

How should we migrate the existing strings?

That's a good question. I see a few options, leaving to Delphine/Pike the choice:

  1. We update en-US and ignore locales, since they're interchangeable (can someone confirm it though?)
  2. We update en-US and fix all other locales by hand in the same PR. That depends on how many strings are affected (I didn't check that), and likely requires a lot of coordination to avoid conflicts.

The sooner we confirm that locales like Japanese are not breaking, and update en-US strings, the better, considering we have a lot of locales starting from scratch with this project.

@Pike
Copy link
Contributor

Pike commented Dec 19, 2019

To be really sure, we need to check all call sites, and see if they're using the API, and not manually replacing %s. Yeah, I know, but we've seen things.

As for updating localizations, if you want to do that on a repo, it'll need to be a PR on android-l10n, not on fenix.

@flodolo
Copy link
Author

flodolo commented Dec 19, 2019

From a quick look, I've only found 2 strings in Fenix using multiple %s

default_device_name
%s on %s %s

search_add_custom_engine_search_string_example
Replace query with “%s”. Example:\nhttps://www.google.com/search?q=%s

Mismatches in locales

default_device_name
ja: %2$s %3$s 上の %1$s
en-US:%s on %s %s

mozac_lib_crash_dialog_button_restart
cs: Restartovat aplikaci %s
en-US:Restart %1$s

default_device_name
zh-CN: %2$s %3$s 上的 %1$s
en-US:%s on %s %s

default_device_name
zh-TW: %2$s %3$s 上的 %1$s
en-US:%s on %s %s

@boek boek added P1 Current sprint and removed needs:group-triage labels Apr 7, 2020
@ekager ekager self-assigned this Apr 10, 2020
ekager added a commit to ekager/fenix that referenced this issue Apr 10, 2020
ekager added a commit to ekager/fenix that referenced this issue Apr 10, 2020
ekager added a commit to ekager/fenix that referenced this issue Apr 13, 2020
@ekager ekager closed this as completed Apr 13, 2020
@liuche liuche mentioned this issue Apr 13, 2020
32 tasks
@liuche liuche mentioned this issue Apr 28, 2020
32 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:health Improve code health 🌐 L10N Localization, translation, strings, .. P1 Current sprint
Projects
None yet
Development

No branches or pull requests

5 participants