Set lang and dir on document.body

RESOLVED FIXED in 1.0

Status

P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

unspecified

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 1.0
(Assignee)

Comment 1

5 years ago
Created attachment 764136 [details] [diff] [review]
patch
Attachment #764136 - Flags: review?(stas)
Blocks: 815900
Comment on attachment 764136 [details] [diff] [review]
patch

Review of attachment 764136 [details] [diff] [review]:
-----------------------------------------------------------------

::: bindings/l20n/html.js
@@ +8,5 @@
>    var localizeHandler;
>    var ctx = L20n.getContext(document.location.host);
>  
> +  // http://www.w3.org/International/questions/qa-scripts
> +  // TODO: dehardcode

File a bug pls about this and set TM: Next?

@@ +56,5 @@
> +    if (documentLocale) {
> +      document.documentElement.lang = documentLocale;
> +      document.documentElement.rtl =
> +        rtlLanguages.indexOf(documentLocale) == -1 ? 'ltr' : 'rtl';
> +    }

This will only be called once, from bootstrap.  Instead, I think we need to move this into localizeHandler and check if l10n.reason tells us the locale just changed.

@@ +117,4 @@
>      var langList = Intl.prioritizeLocales(manifest.languages,
>                                            [navigator.language]);
>      ctx.registerLocales.apply(ctx, langList);
> +    documentLocale = langList[0];

Again, this is called only once.  So far, it was possible to change the language by calling document.l10n.registerLocales('somethingElse').  Do we now need another public method to set the direction?
Attachment #764136 - Flags: review?(stas) → review-
(Assignee)

Comment 3

5 years ago
Created attachment 764156 [details] [diff] [review]
patch v2

updated patch
Attachment #764136 - Attachment is obsolete: true
Attachment #764156 - Flags: review?(stas)
(Assignee)

Comment 4

5 years ago
(In reply to Staś Małolepszy :stas from comment #2)
> File a bug pls about this and set TM: Next?

Bug 884308

> This will only be called once, from bootstrap.  Instead, I think we need to
> move this into localizeHandler and check if l10n.reason tells us the locale
> just changed.

Right. Moved to body of localizeHandler

https://github.com/l20n/l20n.js/commit/12e5e5df9d402df21a2092171363afbef4988190
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Attachment #764156 - Flags: review?(stas) → review+
You need to log in before you can comment on or make changes to this bug.