Add Developer documentation for intl/

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P3
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: zbraniecki, Assigned: zbraniecki)

Tracking

(Blocks 1 bug)

unspecified
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(4 attachments, 4 obsolete attachments)

LocaleService, OSPreferences and Locale with their XPCOM interfaces deserve some documentation.
Blocks: 1346877
Priority: -- → P3
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8962150 [details]
Bug 1438687 - Add Developer documentation for LocaleService.

This piece still requires adding links, but the content should be ready for feedback.
Attachment #8962150 - Flags: feedback?(jfkthame)
Comment on attachment 8962151 [details]
Bug 1438687 - Add Developer documentation for Intl and mozIntl.

Same with this one.
Attachment #8962151 - Flags: feedback?(jfkthame)
Comment on attachment 8962152 [details]
Bug 1438687 - Add Developer documentation for l10n.

I expect that this section requires a complete rewrite, so I treat it more as an entry point to the conversation.

I tried to capture the themes and topics that I think developers would benefit from learning and fit it into the larger document set on Internationalization.

Let's talk about how should this be handled. :)
Attachment #8962152 - Flags: feedback?(francesco.lodolo)
Summary: Add Developer documentation for intl/locale → Add Developer documentation for intl/
Attachment #8962150 - Flags: feedback?(l10n)
Attachment #8962151 - Flags: feedback?(l10n)

Comment 9

Last year
mozreview-review
Comment on attachment 8955029 [details]
Bug 1438687 - Document mozilla::intl::Locale.

https://reviewboard.mozilla.org/r/224210/#review236610

Looks good, if you add a comment for the constructors as well - thanks.

::: intl/locale/MozLocale.h:54
(Diff revision 2)
>      explicit Locale(const nsACString& aLocale);
>      explicit Locale(const char* aLocale)
>        : Locale(nsDependentCString(aLocale))
>        { };

Probably should also document what the constructors expect (i.e. a BCP-47-style locale string), including the fact that they normalize case (and accept `_` as an alternative to `-`).

::: intl/locale/MozLocale.h:78
(Diff revision 2)
> +     */
>      const nsCString AsString() const;
>  
> +    /**
> +     * Compares two locales optionally treating one or both of
> +     * the locales a a range.

s/a a/as a/
Attachment #8955029 - Flags: review?(jfkthame) → review+

Comment 10

Last year
mozreview-review
Comment on attachment 8962152 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/231006/#review236540

Rough first pass at the doc. I think there are a couple of details that we could add, but I wonder what's the role of this MDN page's content
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices

Should it become part of the l10n documentation, and reference from this doc? For example, we talked about picking good l10n-ids in new FTL file, but that choice in not documented anywhere.

::: intl/docs/localization.rst:10
(Diff revision 1)
> +Localization is an aspect of internationalization focused on adapting software to
> +the cultural and regional needs.
> +
> +The boundry between internationalization and localization is fuzzy. At Mozilla
> +we refer to localization when we talk about adapting the user interface
> +and messages, while interantionalization handled operations on data.

typos: internationalization, handles

::: intl/docs/localization.rst:24
(Diff revision 1)
> +At Mozilla localizations are managed by locale communities around the World, who
> +are responsible for maintaining the high quality linguistic and cultural adaptation
> +of Mozilla software into over 100 locales.
> +
> +The exact process of localization management differs from project to project, but
> +in case of Gecko apps the localization is primarely done via a web localization

typo: primarily

::: intl/docs/localization.rst:25
(Diff revision 1)
> +are responsible for maintaining the high quality linguistic and cultural adaptation
> +of Mozilla software into over 100 locales.
> +
> +The exact process of localization management differs from project to project, but
> +in case of Gecko apps the localization is primarely done via a web localization
> +system called Mozilla Pontoon and stored in HG repositories under

nit: Pontoon, not Mozilla Pontoon

::: intl/docs/localization.rst:32
(Diff revision 1)
> +
> +Developers are expected to maintain their code localizable using localization
> +and internationalization systems, and also serve as localizers into the `en-US` locale
> +which is used as the `source` locale.
> +
> +In between those two, there's a sophisticated ecosystem of tools, tests, automation,

I lost the track here: what are "Those two"? Localizers and developers?

::: intl/docs/localization.rst:62
(Diff revision 1)
> +content to be used in the new piece of UI.
> +
> +2) UX mockup + copy review
> +--------------------------
> +
> +This copy is the first step that gets reviewed by the L10n Drivers Team which tries

Two notes: 
* Please break down this sentence so that the reader can breath ;-)
* It's important to note the l10n drivers review the copy for potential localization issues, not the copy itself. That's up to content managers.

::: intl/docs/localization.rst:70
(Diff revision 1)
> +and technical level.
> +
> +3) Patch l10n review
> +--------------------
> +
> +Once that is completed, the next stage is for front end engineers to create patches

front end -> front-end?

::: intl/docs/localization.rst:81
(Diff revision 1)
> +
> +We call this "a social contract" which binds the l10n-id/args combination to a particular
> +source translation and use in the UI.
> +
> +The localizer expects the developer to maintain the contract by ensuring that the
> +translation will be used in the given location and way and will correspond to the

add a comma before "and will"

::: intl/docs/localization.rst:83
(Diff revision 1)
> +source translation and use in the UI.
> +
> +The localizer expects the developer to maintain the contract by ensuring that the
> +translation will be used in the given location and way and will correspond to the
> +source translation. If that contract is to be changed, the developer will be expected
> +to update it. More on that in pkt 6.

pkt?

::: intl/docs/localization.rst:86
(Diff revision 1)
> +translation will be used in the given location and way and will correspond to the
> +source translation. If that contract is to be changed, the developer will be expected
> +to update it. More on that in pkt 6.
> +
> +The next review comes from either L10n Drivers or experienced front end engineers
> +familiar with the internationalization and localization systems making sure that

put "or experienced … systems" between commas

::: intl/docs/localization.rst:90
(Diff revision 1)
> +The next review comes from either L10n Drivers or experienced front end engineers
> +familiar with the internationalization and localization systems making sure that
> +the patches are properly using the right APIs and the code is ready to be landed
> +into `mozilla-central`.
> +
> +4) Exposure in `gecko-strings`

Do we want to talk about the quarantine system?

Also worth noting that a patch landing without review, and with issues, will be likely backed out by sheriffs on l10n-drivers' request unless a quick fix is possible. 

This usually happens before a string hits gecko-strings.

Maybe we should add a link to the gecko-strings repo
https://hg.mozilla.org/l10n/gecko-strings

::: intl/docs/localization.rst:93
(Diff revision 1)
> +into `mozilla-central`.
> +
> +4) Exposure in `gecko-strings`
> +------------------------------
> +
> +Once the patch land, L10n Drivers will take a final look at localizability of the

typo: lands

::: intl/docs/localization.rst:104
(Diff revision 1)
> +
> +From that moment localizers will work on providing translations for the new feature
> +either while the new strings are only in Nightly or after they are merged to Beta.
> +The goal is to have as much of the UI ready in as many locales as early as possible,
> +but that process is continous and we're capable of releasing Firefox with incomplete
> +translations falling back on a backup locale in case of a missing string.

That's not technically true at the moment, we only fall back to en-US.

::: intl/docs/localization.rst:106
(Diff revision 1)
> +either while the new strings are only in Nightly or after they are merged to Beta.
> +The goal is to have as much of the UI ready in as many locales as early as possible,
> +but that process is continous and we're capable of releasing Firefox with incomplete
> +translations falling back on a backup locale in case of a missing string.
> +
> +L10n Drivers team is maintaining the status of each locale and deciding whether

What were you thinking of when writing this paragraph? Sign-offs?

While Nightly products use the latest version of localization available in repositories, the L10n Drivers team is responsible for reviewing and signing off versions of each localization shipping in Beta and Release versions of Gecko products.

::: intl/docs/localization.rst:121
(Diff revision 1)
> +and from the localization resource file in `mozilla-central`.
> +
> +If it's an update, we currently have two "levels" of change severity:
> +
> +1) If the change is minor, like a spelling or punctuation, the developer should update
> +the `en-US` translation.

I'd call out explicitly "without updating the l10n-id".

::: intl/docs/localization.rst:122
(Diff revision 1)
> +
> +If it's an update, we currently have two "levels" of change severity:
> +
> +1) If the change is minor, like a spelling or punctuation, the developer should update
> +the `en-US` translation.
> +2) If the change is anyhow affecting the meaning or tone of the message, the developer

We should explicitly note that, in Fluent, changes to the structure of the string, i.e. adding attributes, requires a new ID as well.

::: intl/docs/localization.rst:129
(Diff revision 1)
> +
> +The latter is considered a change in the social contract between the developer and
> +the localizer and an update to the ID is expected.
> +
> +The new ID will be recognized by the l10n tooling as untranslated, and the old one
> +as obsolete. This will give the localizer oportunnity to find and update the translation.

typo: opportunity

::: intl/docs/localization.rst:131
(Diff revision 1)
> +the localizer and an update to the ID is expected.
> +
> +The new ID will be recognized by the l10n tooling as untranslated, and the old one
> +as obsolete. This will give the localizer oportunnity to find and update the translation.
> +
> +We're currently evaluating a way to separate out two classes of contract changes,

As someone once told me while writing docs (I think Matjaz), I think it's better to cover the current status more than potential changes. I would drop this paragraph entirely.

::: intl/docs/localization.rst:140
(Diff revision 1)
> +
> +Localization Systems
> +====================
> +
> +Gecko has three main localization systems used. Two older ones, and one
> +being currently introduced with an intention of replacing the previous two.

an intention -> the intion

::: intl/docs/localization.rst:166
(Diff revision 1)
> +
> +It's well suited for modern web development cycle, provides a number of localization
> +features including good internationalization model and strong bidirectionality support.
> +
> +Fluent strictly superseeds the old systems and is currently being slowly introduced to
> +Firefox and all other Mozilla products with an intent to end up being the only

with an intent to end up being the only unified -> with the goal to become the only
(In reply to Francesco Lodolo [:flod] (workweek until Mar 30, slower than usual) from comment #10)
> Comment on attachment 8962152 [details]
> Bug 1438687 - Add Developer documentation for l10n.
> 
> https://reviewboard.mozilla.org/r/231006/#review236540
> 
> Rough first pass at the doc. I think there are a couple of details that we
> could add, but I wonder what's the role of this MDN page's content
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/
> Localization_content_best_practices
> 
> Should it become part of the l10n documentation, and reference from this
> doc? For example, we talked about picking good l10n-ids in new FTL file, but
> that choice in not documented anywhere.


If I understand correctly MDN is moving toward holding more of the webdev resources and less of Mozilla-specific docs.

I'd be happy to go for anything that you think is the right target here.

Comment 12

Last year
mozreview-review
Comment on attachment 8962150 [details]
Bug 1438687 - Add Developer documentation for LocaleService.

https://reviewboard.mozilla.org/r/231002/#review239582

This is long, and I admit I stopped this pass around "UI Direction", as I think at that point the document starts to ramble. There's no apparent structure and context in which to read those. Separate pages might improve that.

Generally, I'd prefer the writing to be more concrete at the risk of being wrong or incomplete, rather than abstract and correct. I came across a couple of places where I found only truth, but I couldn't see how somebody would learn the practical impact from that. I've commented on many of them, often with "remove this paragraph".

More inline, some things are just nits and typos, other's ask to shuffle content around or clarify concepts.

::: intl/docs/index.rst:6
(Diff revision 1)
> +====================
> +Internationalization
> +====================
> +
> +Internationalization ("i18n") is a domain of computer science focused on making
> +software accessible accross languages, regions, scripts and cultures.

Introduce the concept of a Locale here as well?

::: intl/docs/locale.rst:11
(Diff revision 1)
> +=================
> +
> +Internationalization requires to recognize in some way what combination of languages,
> +scripts and regional preferences the user wants to format their data into.
> +
> +A combination of those preferences is called a Locale.

Fold these two paragraphs into one? Drop "Internationalization" from the first, and start with "A locale is ..." or so? Right now, this is a sentence that would make any German proud of saving the important bits for last ;-)

::: intl/docs/locale.rst:31
(Diff revision 1)
> +Above examples present language tags with several fields ommitted, which is allowed
> +by the standard.
> +In order to understand the full model it is necessary to look at the more complete data
> +structure of the Locale.
> +
> +Locale data structure consists of four primary fields.

Can we have the definition first, and then explain why the examples above are OK?

::: intl/docs/locale.rst:36
(Diff revision 1)
> +Locale data structure consists of four primary fields.
> +
> + - Language (Example: English - *en*, French - *fr*, Serbian - *sr*)
> + - Script (Example: Latin - *Latn*, Cyrylic - *Cyrl*)
> + - Region (Example: American - *US*, Canadian - *CA*, Russian - *RU*)
> + - Variants (Example: Mac OS - *macos*, Windows - *windows*, Linux - *linux*)

Explain the length, capitilization, and relevant standards here, too?

::: intl/docs/locale.rst:38
(Diff revision 1)
> + - Language (Example: English - *en*, French - *fr*, Serbian - *sr*)
> + - Script (Example: Latin - *Latn*, Cyrylic - *Cyrl*)
> + - Region (Example: American - *US*, Canadian - *CA*, Russian - *RU*)
> + - Variants (Example: Mac OS - *macos*, Windows - *windows*, Linux - *linux*)
> +
> +On top of that a locale may contain:

Empty line here? Right now, this creates two nested <dl>s.

::: intl/docs/locale.rst:62
(Diff revision 1)
> +which can be then serialized into a string: **"sr-Cyrl-RU"**.
> +
> +Locale Fallback Chains
> +======================
> +
> +Locale sensitive operations are always condidered "best-effort". That means that it

s/condidered/considered/

::: intl/docs/locale.rst:87
(Diff revision 1)
> +Due to the imperfections in data matching, all operations on locales should always
> +use a language negotiation algorithm to resolve the best available set of locales
> +based on the list of all available locales and an ordered list of requested locales.
> +
> +Such algorithms may vary in sophistication and number of used strategies. Mozilla's
> +solution is based on a modified logic from RFC5656.

Make "RFC 5656" a link? And with white-space?

::: intl/docs/locale.rst:91
(Diff revision 1)
> +Such algorithms may vary in sophistication and number of used strategies. Mozilla's
> +solution is based on a modified logic from RFC5656.
> +
> +The three lists of locales used in negotiation:
> +
> + - **Available** - locales that can be selected

Maybe make this "are locally installed"?

::: intl/docs/locale.rst:97
(Diff revision 1)
> + - **Requested** - locales that the user selected in decreasing order of preference
> + - **Resolved** - result of the negotiation
> +
> +On top of that there's a concept of *DefaultLocale* which is a single locale out
> +of the list of available ones that should be used as a in case there is no match
> +to be found between available and requested locales.

Let's call out `en-US` and that we choose this because it's the language the software is originally developed in. Without this, this paragraph is too abstract.

::: intl/docs/locale.rst:99
(Diff revision 1)
> +
> +On top of that there's a concept of *DefaultLocale* which is a single locale out
> +of the list of available ones that should be used as a in case there is no match
> +to be found between available and requested locales.
> +
> +A result of a negotiation is an ordered list of locales that are available to the system

"The result..."

::: intl/docs/locale.rst:105
(Diff revision 1)
> +and it is expected for the consumer to attempt using the locales in the resolved order.
> +
> +Negotiation should be used in all scenarios like selecting language resources,
> +calendar, number formattings etc.
> +
> +Filtering / Matching / Lookup

Can you introduce the basics of matching of a single locale with another single locale before this?

::: intl/docs/locale.rst:114
(Diff revision 1)
> +
> +Filtering
> +^^^^^^^^^
> +
> +The first one is called **filtering** and handles a scenario the reader wants to
> +filter get all possible available locales using the requested locales.

This paragraph can be removed.

::: intl/docs/locale.rst:128
(Diff revision 1)
> +    let requested = ["fr-CA", "en-US"]
> +    let available = ["en-GB", "it", "en-ZA", "fr", "de-DE", "fr-CA", "fr-ZH"]
> +
> +    let result = Services.locale.negotiateLanguages(requested, available);
> +
> +    result; // ["fr-CA", "fr", "fr-ZH", "en-GB", "en-ZA"]

Here and below, don't format the outcome as a comment. In the formatted doc, that's light grey, and hardly readable.

::: intl/docs/locale.rst:153
(Diff revision 1)
> +.. code-block:: javascript
> +
> +    let requested = ["fr-CA", "en-US"]
> +    let available = ["en-GB", "it", "en-ZA", "fr", "de-DE", "fr-CA", "fr-ZH"]
> +
> +    let result = Services.locale.negotiateLanguages(requested, available);

Fix the API call here, and in lookup. Might also change the results.

::: intl/docs/locale.rst:181
(Diff revision 1)
> +
> +    result; // ["fr-CA"]
> +
> +
> +Chained Language Negotiation
> +============================

I struggle to understand this whole paragraph.

I'm also not convinced about the assertion that a user requesting a UI locale shouldn't get that in an add-on, even if Firefox doesn't have it.

Is this practically relevant? If not, we should just drop this section to make the docs easier to read.

::: intl/docs/locale.rst:243
(Diff revision 1)
> +Available Locales
> +=================
> +
> +In Gecko, available locales are combined from a list of locale resources
> +packaged into the bundle (see: `Packaged Locales`), and a list of instlaled special
> +addons called `language packs`.

"combine" can mean a lot of things. How about ".. come from the packaged locales and the installed language packs. Language packs are a variant of web extensions providing just localized resources for one or more languages." ?

::: intl/docs/locale.rst:252
(Diff revision 1)
> +
> +In the future Mozilla may want to separate this into different sets of available locales,
> +like locales available for Intl API, installer, component like devtools, hypenations etc.
> +
> +Each addon may also have its own list of locales, but currently Gecko does not store a
> +centralized list of those sets.

These two paragraphs seem to be more confusing than helping, at least to me.

::: intl/docs/locale.rst:257
(Diff revision 1)
> +centralized list of those sets.
> +
> +Requested Locales
> +=================
> +
> +Similarly to available locales, Geceko currently only stores a single list of requested locales.

Drop this paragraph?

::: intl/docs/locale.rst:261
(Diff revision 1)
> +
> +Similarly to available locales, Geceko currently only stores a single list of requested locales.
> +
> +The locale list is stored in a pref `intl.locale.requested`, but for all purposes
> +from within the app should be set/read via `LocaleService::GetRequestedLocales` and
> +`LocaleService::SetRequestedLocales` API, never directly via the pref itself.

Start with the API, end with the pref.

::: intl/docs/locale.rst:268
(Diff revision 1)
> +Using the API will perform necessary sanity checks and canonicalize the values.
> +
> +The pref usually will be used to store a comma separated list of valid BCP47 locale
> +codes, but it can also have two special meanings:
> +
> + - If the pref is not set at all, Gecko will use the default locale as the requested one.

It's the packaged locale, not the default one.

::: intl/docs/locale.rst:269
(Diff revision 1)
> +
> +The pref usually will be used to store a comma separated list of valid BCP47 locale
> +codes, but it can also have two special meanings:
> +
> + - If the pref is not set at all, Gecko will use the default locale as the requested one.
> + - If the pref is set to an empty string, Gecko will look into OS app locales as the requested.

This looks like those would be edge-cases, but both are our defaults, one for Firefox, one for Fennec.

Make that more explicit?

::: intl/docs/locale.rst:295
(Diff revision 1)
> +
> +One is to store them as a separate data structure and provide API to retrieve them.
> +Another is to use Unicode Extension Keys to add such information to the Locale
> +object and in result language tag.
> +An example of a correctly encoded BCP47 language tag for American English with
> +hour cycle specifically set to be *"24"* is **"en-US-u-hc-h24"**.

Remove things we're not doing here?

::: intl/docs/locale.rst:323
(Diff revision 1)
> +is in *"fr-FR"* it won't.
> +In order to enforce Gecko to look into OS preferences irrelevant of the language match,
> +set the flag `intl.locale.use_os_preferences` to `true`.
> +
> +Default and Last Fallback Locales
> +=================================

I don't understand this paragraph. `DefaultLocale` up in this document seems to contradict this paragraph. Also not sure if we're actually using two different locales here?

::: intl/docs/locale.rst:384
(Diff revision 1)
> +
> +Gecko supports the limitation by accepting the 3-letter variants in our APIs and also
> +provide a special `GetAppLocalesAsLangTags` method which returns this locale in that form.
> +(`GetAppLocalesAsBCP47` will canonicalize it and turn into `ja-JP-x-variant-mac`).
> +
> +In the future there's hope this exception will be removed.

Make an assertion here instead. Usage of language negotiation etc shouldn't rely on this behavior.
Attachment #8962150 - Flags: feedback?(l10n) → feedback-

Comment 13

Last year
mozreview-review
Comment on attachment 8962150 [details]
Bug 1438687 - Add Developer documentation for LocaleService.

https://reviewboard.mozilla.org/r/231002/#review240902

::: intl/docs/locale.rst:257
(Diff revision 1)
> +centralized list of those sets.
> +
> +Requested Locales
> +=================
> +
> +Similarly to available locales, Geceko currently only stores a single list of requested locales.

Also, "Geceko" typo

::: intl/docs/locale.rst:351
(Diff revision 1)
> +There is currently work being done on enabling more flexibility in how
> +the locales are packaged to allow for bundling applications with different
> +sets of locales in different areas - dictionaries, hypenations, product language resources,
> +installer language resources, etc.
> +
> +UI Direction

This should be a subsection of Regional Preferences. Not that it's really a region, but it's locale-dependent metadata that you get from intl services.

::: intl/docs/locale.rst:361
(Diff revision 1)
> +
> +`LocaleService::IsAppLocaleRTL` returns a boolean indicating what's the current
> +direction of the app UI.
> +
> +Server / Client
> +===============

This section should be called `Multi-Process`. Server-client is an implementation detail.

::: intl/docs/locale.rst:386
(Diff revision 1)
> +provide a special `GetAppLocalesAsLangTags` method which returns this locale in that form.
> +(`GetAppLocalesAsBCP47` will canonicalize it and turn into `ja-JP-x-variant-mac`).
> +
> +In the future there's hope this exception will be removed.
> +
> +Environments

This section disrupts the flow for me. I wonder if we should split this into a different doc.

::: intl/docs/locale.rst:413
(Diff revision 1)
> +the LocaleService and `mozilla.org/intl/mozIOSPreferences` for OS preferences.
> +
> +The LocaleService API is exposed as `Services.locale` object.
> +
> +There's currently no API available for operations on language tags and Locale objects,
> +but `Intl.Locale` API is being in works.

Is there a bug to link to here?

::: intl/docs/locale.rst:419
(Diff revision 1)
> +
> +Rust
> +----
> +
> +For Rust Mozilla provides a crate `fluent-locale` which implements the concepts described
> +above. This crate is not yet vendored into mozilla-central.

We should probably discourage importing alternative implementations of langneg into m-c? Maybe this should rather talk about the lack of bindgen or something?
If we still want to mention fluent-locale, should that link to https://docs.rs/fluent-locale/ ?

::: intl/docs/locale.rst:421
(Diff revision 1)
> +----
> +
> +For Rust Mozilla provides a crate `fluent-locale` which implements the concepts described
> +above. This crate is not yet vendored into mozilla-central.
> +
> +Events

Make this a section about locale changes? Also, for consistency, don't incentivize to change the pref here ;-)

::: intl/docs/locale.rst:441
(Diff revision 1)
> +
> +Testing
> +=======
> +
> +Many components may have logic encoded to react to changes in requested, available
> +of resolved locales.

First things first, start with "Testing ..."

Also probably a good idea to cut the doc here into a separate page.

Also, s/of/or,/

The whole section also needs to be more constructive. What do you test, when do you use which method, when to use real codes and when to use `x-test`.

::: intl/docs/locale.rst:490
(Diff revision 1)
> +    let appLocales = Services.locale.getAppLocalesAsBCP47();
> +    assert(appLocales[0], "ko-KR");
> +
> +In the future Mozilla plans to add a special, third way for addons, to allow for either
> +manual or automated testing purposes disconnecting its locales from main application
> +ones.

Bug link. Also, this sounds like there's no way to test localized web extensions without having Firefox in that language. Is that currently true?

::: intl/docs/locale.rst:512
(Diff revision 1)
> +In case of localization, it is best to test against the correct `data-l10n-id`
> +being set or in edge cases verify that a given variable is present in the string using
> +`String.prototype.includes`.
> +
> +Startup
> +=======

This needs more structure (and it also worth its own page).

Right now, this is a dozen 1-2 line paragraphs spanning my 24'' portrait-mode monitor.

Maybe starting with the different data classes that affect language selection throughout the startup?

::: intl/docs/locale.rst:580
(Diff revision 1)
> +
> +Summary
> +=======
> +
> +The model of locale management described above has been designed in year 2017 and
> +in result, a lot of older Gecko code may not be well integrated into it.

I'd avoid writing 2017 in here, 'cause in 2030 it's gonna look like crap. Also, this paragraph questions actually doing the intl work ;-)

I'd just replace Summary with "Feedback" or so, and just use the last paragraph.

Comment 14

Last year
mozreview-review
Comment on attachment 8962151 [details]
Bug 1438687 - Add Developer documentation for Intl and mozIntl.

https://reviewboard.mozilla.org/r/231004/#review240968

::: intl/docs/dataintl.rst:15
(Diff revision 1)
> +and so on.
> +
> +Most of the APIs are backed by the CLDR and ICU and the focus is on enabling
> +front-end code internationalization, which means the majority of the APIs are
> +primarely available in JavaScript, with C++ and Rust having only a small subset of
> +them enabled.

"primarily", and "exposed" instead of "enabled"?

::: intl/docs/dataintl.rst:21
(Diff revision 1)
> +
> +Best Practices
> +==============
> +
> +The most important best practice when dealing with data internationalization is to
> +perform it on as high level as possible; right before the UI is displayed.

"high level" probably depends on the developer, meaning different things to a network and a graphics guy. Let's reword this "as close to the actual UI as possible"? Probably merges with the paragraph after.

::: intl/docs/dataintl.rst:43
(Diff revision 1)
> +
> +The above is also important in the context of testing. It is a common mistake to
> +attempt to write tests that verify the output of the UI with internationalized data.
> +
> +The underlying data set used to create the formatted version of the data may and will
> +change over time both, due to dataset improvements and also changes to the language

"over time, both due to...", comma location.

::: intl/docs/dataintl.rst:49
(Diff revision 1)
> +and regional preferences over time.
> +That means that tests that attempt to verify the exact output will require
> +significantly higher level of maintenance and will remain more brittle.
> +
> +Most of the APIs provide special method, like `resolvedOptions` which should be used
> +in stead to verify the output matching the expectations.

Most of the APIs provide special methods like `resolvedOptions`, which should be used
instead to verify that the output is matching the expectations.

::: intl/docs/dataintl.rst:56
(Diff revision 1)
> +JavaScript Internationalization API
> +===================================
> +
> +JavaScript standard comes with a set of APIs in form of a standard called ECMA402.
> +ECMA402 contains a set of APIs designed to enable data internationalization and is
> +supported by all major JS environments.

Data internationalization APIs are formalized in the JavaScript standard in ECMA 402. These APIs are supported by all major JS environments.

Link that up to https://tc39.github.io/ecma402/ ?

::: intl/docs/dataintl.rst:58
(Diff revision 1)
> +
> +JavaScript standard comes with a set of APIs in form of a standard called ECMA402.
> +ECMA402 contains a set of APIs designed to enable data internationalization and is
> +supported by all major JS environments.
> +
> +It is best to consult the MDN article on the current state of the Intl API.

MDN article should be a link.

::: intl/docs/dataintl.rst:61
(Diff revision 1)
> +supported by all major JS environments.
> +
> +It is best to consult the MDN article on the current state of the Intl API.
> +Mozilla has an excellent support of the API and relies on it for majority
> +of its needs, through when possible when working on Firefox UI the `mozIntl` wrapper
> +should be used.

Mozilla has an excellent support of the API and relies on it for majority
of its needs. Yet, when working on Firefox UI the `mozIntl` wrapper
should be used.

::: intl/docs/dataintl.rst:64
(Diff revision 1)
> +Mozilla has an excellent support of the API and relies on it for majority
> +of its needs, through when possible when working on Firefox UI the `mozIntl` wrapper
> +should be used.
> +
> +MozIntl
> +=======

I'm not fond of talking `MozIntl` and `mozIntl` here so much. That's more implementation detail, right?

The developer-facing name is `Services.intl`, I suggest to use that where needed.

::: intl/docs/dataintl.rst:93
(Diff revision 1)
> +
> +But that leaves a lot of bits of information that could inform the formatting out.
> +
> +Public API such as `Intl.*` will not be able to look into the Operating System for
> +regional preferences. It will also respect settings such as `resist Fingerprinting`
> +by masking its timezone and locale settings.

This paragraph should come a lot earlier.

The whole section here can use a make-over based on our unpriviledged content strategy, "choose wisely if you use `Intl` or `Services.intl`, based on whether the generated string is accessible from the web for finger-printing.

::: intl/docs/dataintl.rst:135
(Diff revision 1)
> +----------------------
> +
> +`DateTimeFormat` in `mozIntl` gets additional options that provide greater
> +simplicy and consistency to the API.
> +
> +* `timeStyle` and `dateStyle` can take values `short`, `medium`, `long` and `full`

This sentence lacks a period.

::: intl/docs/dataintl.rst:202
(Diff revision 1)
> +      locale,          // "pl"
> +    } = Services.intl.getCalendarInfo();
> +
> +
> +mozIntl.getDisplayNames(locales, options)
> +-----------------------------------------

`boxOfChocolates` ? Are those keys defined anywhere?

::: intl/docs/dataintl.rst:210
(Diff revision 1)
> +internationalization API. Currently the focus is on terms used in date and time
> +formatting, but the keys can be extended.
> +
> +The API takes a locale fallback chain list, and an options object which can contain
> +two keys:
> +* `style` which can takes values `short`, `medium`, `long`

Needs an empty line for list to be a list.

::: intl/docs/dataintl.rst:263
(Diff revision 1)
> +      direction: // "ltr"
> +    } = Services.intl.getLocaleInfo(undefined);
> +
> +
> +mozIntl.RelativeTimeFormat(locales, options)
> +--------------------------------------------

There's been a couple of grammar bugs here, didn't list them in detail.

We should also be explicit about the usability of this api in grammatical contexts.

::: intl/docs/dataintl.rst:303
(Diff revision 1)
> +Additional APIs
> +===============
> +
> +If you find yourself in the need of additional internationalization APIs not currently
> +supported, you can verify if the API proposal is already in the works here,
> +and file a bug in the component X to indicate the request for it.

s/X/Foo/ and link ;-)
Attachment #8962151 - Flags: feedback?(l10n) → feedback-

Comment 15

Last year
mozreview-review
Comment on attachment 8962152 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/231006/#review242946

::: intl/docs/localization.rst:140
(Diff revision 1)
> +
> +Localization Systems
> +====================
> +
> +Gecko has three main localization systems used. Two older ones, and one
> +being currently introduced with an intention of replacing the previous two.

ehm, meant "the intention"
Comment on attachment 8962152 [details]
Bug 1438687 - Add Developer documentation for l10n.

There a few things that are unclear, but it looks like a good start to me.
Attachment #8962152 - Flags: feedback?(francesco.lodolo) → feedback+
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Attachment #8955029 - Attachment is obsolete: true
Attachment #8962150 - Attachment is obsolete: true
Attachment #8962150 - Flags: feedback?(jfkthame)
Attachment #8962151 - Attachment is obsolete: true
Attachment #8962151 - Flags: feedback?(jfkthame)
Attachment #8962152 - Attachment is obsolete: true
Assignee

Comment 21

Last year
mozreview-review-reply
Comment on attachment 8962150 [details]
Bug 1438687 - Add Developer documentation for LocaleService.

https://reviewboard.mozilla.org/r/231002/#review239582

> I struggle to understand this whole paragraph.
> 
> I'm also not convinced about the assertion that a user requesting a UI locale shouldn't get that in an add-on, even if Firefox doesn't have it.
> 
> Is this practically relevant? If not, we should just drop this section to make the docs easier to read.

I'm not sure how to help here. The paragraph is important and has been written in response to a pretty long evaluation between Activity Stream, UX and us.
The decision has been made to make all parts of Firefox UI including extensions follow Firefox UI locale selection as close to possible.

> I don't understand this paragraph. `DefaultLocale` up in this document seems to contradict this paragraph. Also not sure if we're actually using two different locales here?

I tried to clarify that in the section where I introduce the concept and here. We may be using two different locales here, because last fallback is currently hardcoded to `en-US`, while default differs between different locale builds (and matches `update.locale`).
Assignee

Comment 22

Last year
mozreview-review-reply
Comment on attachment 8962151 [details]
Bug 1438687 - Add Developer documentation for Intl and mozIntl.

https://reviewboard.mozilla.org/r/231004/#review240968

> `boxOfChocolates` ? Are those keys defined anywhere?

Not really beyond that. I took this from the header: https://searchfox.org/mozilla-central/source/js/src/builtin/intl/IntlObject.h#69
Assignee

Comment 23

Last year
mozreview-review-reply
Comment on attachment 8962152 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/231006/#review236540

> Do we want to talk about the quarantine system?
> 
> Also worth noting that a patch landing without review, and with issues, will be likely backed out by sheriffs on l10n-drivers' request unless a quick fix is possible. 
> 
> This usually happens before a string hits gecko-strings.
> 
> Maybe we should add a link to the gecko-strings repo
> https://hg.mozilla.org/l10n/gecko-strings

I'd be happy to place more info here, but I don't feel confident to write it. If you'd like to see more data in this iteration, can you provide me with the paragraph please? :)
Ugh, this has been *really* hard to work on. I struggle to understand if it's my fault or just a limitation of writings docs using patches :(

I tried to apply as much feedback as I could and I'd like to do whatever is possible to prevent this from not landing for another couple months if possible.

I think that in order to achieve that I'd need the reviewers to provide me with concrete improvements to paragraphs where possible and leave more high-level refactors for a follow-up update to the docs that can be landed soon after.

If some part is completely not viable for landing, then maybe we can remove it for now and land later?

Alternatively, maybe we should ditch bugs and instead put it in a google docs and work on it there?
I'm not sure, but I hope my reviewers will help me pick the right way forward :)

To sum up. I read and re-read this over and over, came back to it so many times that I lost count, but I don't think it reads "great" as in the same meaning that "great is the enemy of good". It's very easy for me to put it aside and focus on code, and it takes a lot of time to apply feedback.
I'm seeking general review to make sure that the text is understandable and factually correct, and can be extended/improved in the future. I'm also seeking feedback on what to do next if the reviewers won't find the code landable, because the current approach doesn't feel sustainable for the patch author :)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 28

Last year
mozreview-review
Comment on attachment 8981044 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/247164/#review253250

I'd like to see one more iteration of this, but the content looks good.

I feel your pain of using mozreview for docs. We tried Google Docs, and I feel it works better to get to usable content, with the caveat of messing up formatting.

::: intl/docs/localization.rst:8
(Diff revision 2)
> +
> +============
> +Localization
> +============
> +
> +Localization is an aspect of internationalization focused on adapting software to

```
Localization – sometimes written as l10n, where 10 is the number of letters between `l` and `n` – is …
```

::: intl/docs/localization.rst:10
(Diff revision 2)
> +Localization
> +============
> +
> +Localization is an aspect of internationalization focused on adapting software to
> +different cultural and regional needs.
> +

I would add a short definition for internationalization before talking about i18n vs l10n. W3C has a pretty good one IMO.
https://www.w3.org/International/questions/qa-i18n/qa-i18n.ar.php

Internationalization is the design and development of a product that enables easy localization for target audiences that vary in culture, region, or language.

::: intl/docs/localization.rst:11
(Diff revision 2)
> +============
> +
> +Localization is an aspect of internationalization focused on adapting software to
> +different cultural and regional needs.
> +
> +The boundry between internationalization and localization is fuzzy. At Mozilla

typo: boundary

::: intl/docs/localization.rst:46
(Diff revision 2)
> +Content vs. UI
> +==============
> +
> +The two main categories in localization are content localization vs UI localization.
> +
> +The former is usually involved when dealing with large blocks of texts such as

blocks of text (singular)

::: intl/docs/localization.rst:47
(Diff revision 2)
> +==============
> +
> +The two main categories in localization are content localization vs UI localization.
> +
> +The former is usually involved when dealing with large blocks of texts such as
> +documentation, help articles, marketing materials and legal documents.

material (singular)

::: intl/docs/localization.rst:67
(Diff revision 2)
> +content to be used in the new piece of UI.
> +
> +2) UX mockup + copy review
> +--------------------------
> +
> +The UX mockup with copy is the first step that gets reviewed by the L10n Drivers Team.

I realize "that gets reviewed" doesn't really match reality though. Maybe we should say "that should get"?

::: intl/docs/localization.rst:98
(Diff revision 2)
> +into `mozilla-central`.
> +
> +4) Exposure in `gecko-strings`
> +------------------------------
> +
> +Once the patch lands, L10n Drivers will take a final look at localizability of the

Let's rewrite this section

```
Once the patch lands in `mozilla-central`, L10n Drivers will take a final look at the localizability of the introduced strings. In case of issues, developers might be asked to land a follow up, or the patch could be backed out with the help of sheriffs.

Every few days, strings are exported into a repository called `gecko-strings-quarantine`, a unified repository that includes strings for all shipping versions of Firefox (nightly, beta, release). This repository is used as a buffer to avoid exposing potential issues to over 100 locales.

As a last step, strings are pushed into `gecko-strings`, another unified repository that is exposed to localization tools, like Pontoon, and build automation.
```

::: intl/docs/localization.rst:119
(Diff revision 2)
> +
> +6) String updates
> +-----------------
> +
> +If later in the software engineering life cycle the need arises to update or remove
> +the string, the l10n review comes again.

Let's rephrase the first paragraph

```
Later in the software life cycle some strings might need to be changed or removed. As a general rule, once the strings lands in `mozilla-central`, any further update to existing strings will need to follow these guidelines, independently from how much time has passed since previous changes.
```

::: intl/docs/localization.rst:126
(Diff revision 2)
> +If it's just a string removal, all the engineer has to do is to remove it from the UI
> +and from the localization resource file in `mozilla-central`.
> +
> +If it's an update, we currently have two "levels" of change severity:
> +
> +1) If the change is minor, like a spelling or punctuation, the developer should update

"like fixing a spelling error or case"

Punctuation is not a good example for this case.

::: intl/docs/localization.rst:135
(Diff revision 2)
> +is requested to update the l10n string ID.
> +
> +The latter is considered a change in the social contract between the developer and
> +the localizer and an update to the ID is expected.
> +
> +In case of `Fluent`_, any changes to the structure of the message such as adding/removing

typo: requires

::: intl/docs/localization.rst:139
(Diff revision 2)
> +
> +In case of `Fluent`_, any changes to the structure of the message such as adding/removing
> +attributes also require an update of the ID.
> +
> +The new ID will be recognized by the l10n tooling as untranslated, and the old one
> +as obsolete. This will give the localizer opportunity to find and update the translation.

```
This will give the localizers an opportunity to find and update the translation, while the old string will be removed from the build process.

There is a gray area between the two severity levels. In case of doubt, don’t exitate to request feedback of review from L10n Drivers to avoid issues once the strings land.
```

::: intl/docs/localization.rst:153
(Diff revision 2)
> +
> +An l10n identifier, once defined, is then getting associated to a translated
> +message in every one of 100+ locales and it becomes very costly to attempt to
> +migrate that string in all locales to a different identifier.
> +
> +Additionally, an identifier is used as a last resort string to be displayed in

Add that this is the case for Fluent.

::: intl/docs/localization.rst:161
(Diff revision 2)
> +
> +Lastly, l10n resources get mixed and matched into localization contexts where
> +it becomes important to avoid identifier collision from two strings coming
> +from two different files.
> +
> +For all those reasons, a longer identifier such as :js:`privacy-exceptions-button-ok` are

is preferred

::: intl/docs/localization.rst:167
(Diff revision 2)
> +preferred over short identifiers like :js:`ok` or :js:`ok-button`.
> +
> +Localization Systems
> +====================
> +
> +Gecko has three main localization systems. Two older ones, and one

Gecko has three main localization systems: two older ones (DTD and StringBundle) and a new system, called Fluent, that is progressively replacing them.

::: intl/docs/localization.rst:173
(Diff revision 2)
> +being currently introduced with the intention of replacing the previous two.
> +
> +DTD & StringBundle
> +------------------
> +
> +DTD is primarely used for XUL and XHTML file localization. It uses `.dtd` files

typo: primarily

::: intl/docs/localization.rst:174
(Diff revision 2)
> +
> +DTD & StringBundle
> +------------------
> +
> +DTD is primarely used for XUL and XHTML file localization. It uses `.dtd` files
> +and the only localization feature it provides is ability to reference one

the ability

::: intl/docs/localization.rst:177
(Diff revision 2)
> +
> +DTD is primarely used for XUL and XHTML file localization. It uses `.dtd` files
> +and the only localization feature it provides is ability to reference one
> +string from another via entity reference.
> +
> +StringBundle is a runtime API used primarely for localization of the JS code.

typo: primarily

Comment 29

Last year
mozreview-review
Comment on attachment 8981041 [details]
Bug 1438687 - Document mozilla::intl::Locale.

https://reviewboard.mozilla.org/r/247158/#review253280

::: intl/locale/MozLocale.h:63
(Diff revision 1)
> +     *
> +     *  * Case normalization to conform with BCP47 (e.g. "eN-uS" -> "en-US")
> +     *  * Underscore delimiters replaced with dashed (e.g. "en_US" -> "en-US")
> +     *
> +     * If the input language tag string is not well-formed, the Locale will be
> +     * created with its flag `mValid` set to false.

Drive by comment: we don't actually check that the language tag is valid (i.e., in the subtag registry), but only well-formed, but call it valid?

Should we rename valid to wellformed? I'd spin that off into a new bug.

::: intl/locale/MozLocale.h:128
(Diff revision 1)
> +     */
>      void ClearRegion();
>  
> -    // Mark the object as invalid, meaning we shouldn't use it any more.
> +    /**
> +     * Marks the locale as invalid which in turns makes
> +     * it to be skipped by most LocaleService operations.

I'd move this up and clarify the "most".

You're using IsValid() for two different use-cases, right? One for actually junk pref data that's not well-formed, and the other is to mark locales as already handled?

And that works by ::Matches always returning false if either Locale object is marked as invalid?

Something like "Only valid Locale objects can ever match. Locales that are not well-formed are always invalid. Algorithms may use this to mark Locales as done, too."

Going back to my first comment, maybe the flag is better named `hidden`? In a different bug, that is.

Comment 30

Last year
mozreview-review
Comment on attachment 8981044 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/247164/#review253314

::: intl/docs/localization.rst:67
(Diff revision 2)
> +content to be used in the new piece of UI.
> +
> +2) UX mockup + copy review
> +--------------------------
> +
> +The UX mockup with copy is the first step that gets reviewed by the L10n Drivers Team.

Actually: probably better "that could be"
Assignee

Comment 31

Last year
mozreview-review-reply
Comment on attachment 8981041 [details]
Bug 1438687 - Document mozilla::intl::Locale.

https://reviewboard.mozilla.org/r/247158/#review253280

> I'd move this up and clarify the "most".
> 
> You're using IsValid() for two different use-cases, right? One for actually junk pref data that's not well-formed, and the other is to mark locales as already handled?
> 
> And that works by ::Matches always returning false if either Locale object is marked as invalid?
> 
> Something like "Only valid Locale objects can ever match. Locales that are not well-formed are always invalid. Algorithms may use this to mark Locales as done, too."
> 
> Going back to my first comment, maybe the flag is better named `hidden`? In a different bug, that is.

> and the other is to mark locales as already handled?

I don't think we do that. mIsValid (and IsValid()) are only marking if the language tag is well formed. I agree we may want to rename it to IsWellFormed. :)
Assignee

Comment 32

Last year
mozreview-review-reply
Comment on attachment 8981044 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/247164/#review253250

> I would add a short definition for internationalization before talking about i18n vs l10n. W3C has a pretty good one IMO.
> https://www.w3.org/International/questions/qa-i18n/qa-i18n.ar.php
> 
> Internationalization is the design and development of a product that enables easy localization for target audiences that vary in culture, region, or language.

So, this is a subsection of a module called Internationalization. The definition of Internationalization is in the index.rst which links to this document. I'd prefer to avoid explaining what intl is in every subdocument separately.

Let me know if you think we still should.
Assignee

Comment 33

Last year
mozreview-review-reply
Comment on attachment 8981044 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/247164/#review253314

> Actually: probably better "that could be"

I put "that should be" :) Let's incentivise good culture :)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
> I feel your pain of using mozreview for docs. We tried Google Docs, and I feel it works better to get to usable content, with the caveat of messing up formatting.

The issue with gdocs is that I feel like part of the narrative flow when I write it is examples, notes, important sections and links. With gdocs I just write a block of text and need to overexplain instead of show.

But yeah, it's really tricky to write docs, especially big ones. My hope is that updating/extending them is way easier in my experience so once we land this it should be relatively smooth to keep improving it over time. It's just the initial landing that sucks. :)
https://dxr.mozilla.org/mozilla-central/rev/a466172aed4bc2afc21169b749b8068a4b98c93f/intl/locale/LocaleService.cpp#454,471 are the two cases that I had found yesterday that made me write that. Not that I fully reverse-engineered it, but that seems to marking well-formed locales (thankfully their copies) as invalid?
> Not that I fully reverse-engineered it, but that seems to marking well-formed locales (thankfully their copies) as invalid?

Oh, hah. Thanks for bringing them up! That's a workaround Jonathan put in place in bug 1434169 which I completely forgot about (shame on me, I reviewed them!). Frankly, I'd prefer to find a better workaround over legitimizing use of `mIsValid` as a way to mark matches of well-formed locales.

Is it ok if I file a bug to fix it instead of documenting this use case?
Yeah.

Comment 42

Last year
mozreview-review
Comment on attachment 8981044 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/247164/#review253448

It looks good to me. I'd say let's land this, and iterate as needed if we want to improve.
Attachment #8981044 - Flags: review?(francesco.lodolo) → review+

Comment 43

Last year
mozreview-review
Comment on attachment 8981044 [details]
Bug 1438687 - Add Developer documentation for l10n.

https://reviewboard.mozilla.org/r/247164/#review253490
Attachment #8981044 - Flags: review?(l10n) → review+

Comment 44

Last year
mozreview-review
Comment on attachment 8981043 [details]
Bug 1438687 - Add Developer documentation for Intl and mozIntl.

https://reviewboard.mozilla.org/r/247162/#review253492
Attachment #8981043 - Flags: review?(l10n) → review+

Comment 45

Last year
mozreview-review
Comment on attachment 8981042 [details]
Bug 1438687 - Add Developer documentation for LocaleService.

https://reviewboard.mozilla.org/r/247160/#review253494
Attachment #8981042 - Flags: review?(l10n) → review+

Comment 46

Last year
mozreview-review
Comment on attachment 8981041 [details]
Bug 1438687 - Document mozilla::intl::Locale.

https://reviewboard.mozilla.org/r/247158/#review253502

Snatching this review from Jonathan. I checked that his comment 9 made it here, and checked the new constructor docs.
Attachment #8981041 - Flags: review+
(In reply to Francesco Lodolo [:flod] from comment #42)
> It looks good to me. I'd say let's land this, and iterate as needed if we
> want to improve.

Same here

Comment 48

Last year
mozreview-review
Comment on attachment 8981041 [details]
Bug 1438687 - Document mozilla::intl::Locale.

https://reviewboard.mozilla.org/r/247158/#review253634

::: intl/locale/MozLocale.h:76
(Diff revision 2)
>      const nsACString& GetScript() const;
>      const nsACString& GetRegion() const;
>      const nsTArray<nsCString>& GetVariants() const;
>  
> +    /**
> +     * Returns a `true` if the locale is valid, or `false` if it is not.

As :pike noted, we don't check *validity* in a BCP47 sense, we only require *well-formedness*. I'm not sure how I feel about renaming the variable/API to reflect this (it gets a bit cumbersome), but it's probably worth clarifying in the comment here that "valid" only means "well-formed, such that the Locale object can validly be matched against others", and does not require the locale to be BCP47-valid.
Attachment #8981041 - Flags: review?(jfkthame) → review+

Comment 49

Last year
mozreview-review
Comment on attachment 8981042 [details]
Bug 1438687 - Add Developer documentation for LocaleService.

https://reviewboard.mozilla.org/r/247160/#review253638

LGTM, a few minor typos noted.

::: intl/docs/locale.rst:38
(Diff revision 3)
> +their capitalization and order in which the fields should be defined.
> +
> +Most of the base subtags are valid ISO codes, such as `ISO 639`_ for
> +language subtag, or `ISO 3166-1`_ for region.
> +
> +The examples above present language tags with several fields ommitted, which is allowed

s/ommitted/omitted/

::: intl/docs/locale.rst:81
(Diff revision 3)
> +
> +Locale sensitive operations are always considered "best-effort". That means that it
> +cannot be assumed that a perfect match will exist between what the user requested and what
> +the API can provide.
> +
> +In result, the best practice is to *always* operate on locale fallback chains -

s/In result/As a result/

::: intl/docs/locale.rst:87
(Diff revision 3)
> +ordered lists of locales according to the user preference.
> +
> +An example of a locale fallback chain may be: :js:`["es-CL", "es-ES", "es", "fr", "en"]`.
> +
> +The above means a request to format the data according to the Chilean spanish if possible,
> +fall back on Spanish spanish, then any (generic) spanish, french and eventually on

s/on/to/ (twice in this line)

::: intl/docs/locale.rst:181
(Diff revision 3)
> +    let requested = ["fr-CA", "en-US"];
> +    let available = ["en-GB", "it", "en-ZA", "fr", "de-DE", "fr-CA", "fr-ZH"];
> +
> +    let result = Services.locale.negotiateLanguages(requested, available);
> +
> +    result == ["fr-CA", "fr", "fr-ZH", "en-GB", "en-ZA"];

s/fr-ZH/fr-CH/, here and above

::: intl/docs/locale.rst:272
(Diff revision 3)
> +
> +For example, a Firefox extension may come with its own list of available locales, which
> +may have locales that Firefox doesn't.
> +
> +In that case, negotiation between user requested locales and the addon's list may result
> +in a selection of locales superceeding that of Firefox itself.

s/superceeding/superseding/

::: intl/docs/locale.rst:415
(Diff revision 3)
> +If all else fails, Gecko also support a notion of last fallback locale, which is
> +currently hardcoded to *"en-US"* and is the very final locale to try in case
> +nothing else (including the default locale) works.
> +Notice that Unicode and ICU use *"en-GB"* in that role because more English speaking
> +people around the World recognize British regional preferences than American (metric vs.
> +imperial, farenheit vs celsius etc.).

s/farenheit/fahrenheit/

::: intl/docs/locale.rst:463
(Diff revision 3)
> +======
> +
> +:js:`LocaleService` emits two events: :js:`intl:app-locales-changed` and
> +:js:`intl:requested-locales-changed` which all code can listen to.
> +
> +Those events may be broadcasted in result of new language packs being installed, or

s/in result/as a result/
or
s/in result/in response/

::: intl/docs/locale.rst:476
(Diff revision 3)
> +=======
> +
> +Many components may have logic encoded to react to changes in requested, available
> +or resolved locales.
> +
> +In order to test the components behavior, it is important to replicate

s/components/component's/
Attachment #8981042 - Flags: review?(jfkthame) → review+

Comment 50

Last year
mozreview-review
Comment on attachment 8981043 [details]
Bug 1438687 - Add Developer documentation for Intl and mozIntl.

https://reviewboard.mozilla.org/r/247162/#review253644

::: intl/docs/dataintl.rst:72
(Diff revision 3)
> +regional preferences. It will also respect settings such as `Resist Fingerprinting`
> +by masking its timezone and locale settings.
> +
> +This is a fair tradeoff when dealing with the Web Content, but in most cases, the
> +privileged UI of the Gecko application should be able to access all of those
> +additional bits and not be affected by the anti-fingerpting masking.

s/fingerpting/fingerprinting/
Attachment #8981043 - Flags: review?(jfkthame) → review+
Assignee

Comment 51

Last year
mozreview-review-reply
Comment on attachment 8981041 [details]
Bug 1438687 - Document mozilla::intl::Locale.

https://reviewboard.mozilla.org/r/247158/#review253634

> As :pike noted, we don't check *validity* in a BCP47 sense, we only require *well-formedness*. I'm not sure how I feel about renaming the variable/API to reflect this (it gets a bit cumbersome), but it's probably worth clarifying in the comment here that "valid" only means "well-formed, such that the Locale object can validly be matched against others", and does not require the locale to be BCP47-valid.

I changed the description to what you suggested. The remaining work/discussion will happen in bug 1465042.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 56

Last year
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/56bf1eeea50a
Document mozilla::intl::Locale. r=jfkthame,Pike
https://hg.mozilla.org/integration/autoland/rev/38922a89e9d5
Add Developer documentation for LocaleService. r=jfkthame,Pike
https://hg.mozilla.org/integration/autoland/rev/e9b7d0f51d86
Add Developer documentation for Intl and mozIntl. r=jfkthame,Pike
https://hg.mozilla.org/integration/autoland/rev/50c588f317f4
Add Developer documentation for l10n. r=flod,Pike
Thank you so much Axel, Francesco, Jonathan! So happy to land it and feel free to improve it as you see fit. I claim no ownership over those docs :)
You need to log in before you can comment on or make changes to this bug.