If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Status

()

Firefox for iOS
Localization
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bnicholson, Assigned: bnicholson)

Tracking

unspecified
All
iOS

Firefox Tracking Flags

(fxios-v4.0 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Since we're switching to NSLocalizedString with keys, we should name the IDs consistently across the app. We can make this easier if group all strings into a single class so they're all lined up together.

An added benefit is that we can also share strings in the cases where it makes sense to do so (e.g., as a settings item, and then as the title for that setting's subscreen).
(Assignee)

Comment 1

2 years ago
flod pointed out that we may still want separate strings for settings items and subsection titles in https://github.com/mozilla/firefox-ios/pull/1683, so ignore point 2 above.
(Assignee)

Comment 2

2 years ago
Created attachment 8737967 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1686

Would like to get strings figured out before the 4.0 freeze. Here's one approach -- curious to hear everyone's thoughts!
Attachment #8737967 - Flags: review?(sleroux)
Attachment #8737967 - Flags: review?(sarentz)
Attachment #8737967 - Flags: review?(jhugman)
Attachment #8737967 - Flags: review?(etoop)
Attachment #8737967 - Flags: review?(bmunar)
Comment on attachment 8737967 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1686

I personally think this is a good idea because of the reasons you said. It makes bookkeeping the strings a lot cleaner (as long as we label the strings per section (e.g. Top Sites, History Panel, etc.) and maintain a consistent format). It does take a little more work to add a new string, like you said, but I think it's worth it than just throwing a bunch of NSLocalizedStrings into classes, some assigned to a variable and some not, etc. The work it takes to maintain the cleanliness is worth it in the long run! (and good and helpful for the l10n team too, I presume)
Attachment #8737967 - Flags: review?(bmunar) → review+
Comment on attachment 8737967 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1686

Yup - I like it! Only thing I would add is when we start moving strings over to the Strings.swift file we can 'group' them using extensions to clean it up a bit instead of comments.
Attachment #8737967 - Flags: review?(sleroux) → review+
(Assignee)

Comment 5

2 years ago
(In reply to Stephan Leroux [:sleroux] from comment #4)
> Yup - I like it! Only thing I would add is when we start moving strings over
> to the Strings.swift file we can 'group' them using extensions to clean it
> up a bit instead of comments.

Good idea! Updated the PR.

Merging so that we can use String.swift for incoming PRs with string changes. I think the most important thing is that we agree on a format for l10n string IDs; we can always change the Swift code later if we want.

https://github.com/mozilla/firefox-ios/commit/7f7b334dd6201679ec10afc6c80a6de037b852dd
(Assignee)

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-fxios-v4.0: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 4.0
(Assignee)

Comment 6

2 years ago
Comment on attachment 8737967 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1686

I'll also update the README with details about using NSLocalizedString with value and string IDs.
Attachment #8737967 - Flags: review?(sarentz)
Attachment #8737967 - Flags: review?(jhugman)
Attachment #8737967 - Flags: review?(etoop)
NI on Stefan in case he's the one doing the export for v4.0

As a consequence of this PR, a lot of strings move around, because an entire file moves from 'Client/LightweightThemes.strings' to 'Shared/Supporting Files/LightweightThemes.strings'.

You can get an idea by looking at my diff between v3 and v4
https://github.com/flodolo/fxios-italian/commit/771feca59351c2e8defc7cc49fafa54a6fe425b9

Two issues:
* If these strings are actively used in v3.0, once we export the strings for v4.0 we basically lose them for any v3.x
* If we do this, we need to run a script that rename the file for all locales, and only after that we add new strings. Otherwise, the update script will lose all of the existing translations (the script stores translation by file:id).
Flags: needinfo?(sarentz)
(Assignee)

Updated

2 years ago
Depends on: 1262945
(Assignee)

Comment 8

2 years ago
Thanks for catching this. Going to move these strings back in bug 1262945.
Flags: needinfo?(sarentz)
You need to log in before you can comment on or make changes to this bug.