1507595 - Convert about:support to Fluent

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P3)

Description

[Collin Wing]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.77 Safari/537.36

Comment 1

[Collin Wing]

Attachment: Bug 1507595 Convert about:support to Fluent

Comment 2

[Richard Marti (:Paenglab)]

Comment on attachment 9025531 [details]
Bug 1507595 Convert about:support to Fluent

Things I found during porting to TB:

+            <th data-l10n-id="security-softawre-type"/>

Typo, this should be security-software-type.

-      <h2 class="major-section">
-        &aboutSupport.graphicsTitle;
-      </h2>
+      <h2 class="major-section" data-l10n-name="graphics-title"/>

This should be data-l10n-id.

+            <th colspan="2" class="title-column" data-l10n-id="graphics-crash-guard-title"/>

This should be graphics-crash-guards-title.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1507595 - Convert about:support to Fluent. r?Gijs

Initial patch by Collin Wing ([e-mail]).

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1507595 - Use data-l10n-id instead of formatValue where the change is relatively straightforward. r?Gijs

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28805dd1b97a
Convert about:support to Fluent. r=Gijs,flod
https://hg.mozilla.org/integration/autoland/rev/485ea25a3b82
Use data-l10n-id instead of formatValue where the change is relatively straightforward. r=Gijs

Comment 6

[Andrei Ciure[:aciure]]

Backed out 2 changesets (Bug 1507595) for Android build bustages, missing aboutSupport.dtd

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=485ea25a3b82fcf3136bd6102313f76e8d955ef4

failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=218667868&searchStr=android%2C4.2%2Cx86%2Copt%2Cbuild-android-x86%2Fopt%2C%28b%29

backout: https://hg.mozilla.org/integration/autoland/rev/8c35900a1c7291259f88caafc8a1f5a9345902d1

Comment 7

[Andreea Pavel [:apavel]]

There are also other failures caused by this bug:

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&failure_classification_id=2&fromchange=49a28ec78bfef405d2a3dc9214bbd8c15cdf4c9a

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1507595 - Test fixes. r?Gijs,flod

There are multiple fixes contained in this patch:
- fixes for Android/mobile support
- adding extra strings to the ftl file that were never defined before, and previously the ID was just printed to the page
- update test_l10n.py to not rely on about:support anymore
- changing Troubleshoot.jsm to return and ID and args for the strings that should be displayed, so it is compatible with the Fluent API
- misc. fixes so strings that are not localized don't go through the Fluent codepath

Comment 9

[Jorg K (CEST = GMT+2)]

Should you be checking attributes["data-l10n-args"] before accessing it here:
https://hg.mozilla.org/integration/autoland/rev/28805dd1b97a#l3.809

+    if (attributes["data-l10n-id"]) {
+      document.l10n.setAttributes(elt,
+                                  attributes["data-l10n-id"],
+                                  attributes["data-l10n-args"]);
+      delete attributes["data-l10n-id"];
+      if (attributes["data-l10n-args"]) {
+        delete attributes["data-l10n-args"];
+      }

We're porting this to TB and getting:
ReferenceError: reference to undefined property "data-l10n-args"
That's with dom.report_all_js_exceptions and/or javascript.options.strict set.

Oops, can't set NI since it's already set.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Jorg K (GMT+1) (urgent reviews and bustage fix only, Dec 22nd to Jan 1st) from comment #9)
> Should you be checking attributes["data-l10n-args"] before accessing it here:
> https://hg.mozilla.org/integration/autoland/rev/28805dd1b97a#l3.809
> 
> +    if (attributes["data-l10n-id"]) {
> +      document.l10n.setAttributes(elt,
> +                                  attributes["data-l10n-id"],
> +                                  attributes["data-l10n-args"]);
> +      delete attributes["data-l10n-id"];
> +      if (attributes["data-l10n-args"]) {
> +        delete attributes["data-l10n-args"];
> +      }
> 
> We're porting this to TB and getting:
> ReferenceError: reference to undefined property "data-l10n-args"
> That's with dom.report_all_js_exceptions and/or javascript.options.strict
> set.
> 
> Oops, can't set NI since it's already set.

I wasn't testing with javascript.options.strict, and this code works fine with undefined being returned from the getter. I'll make the change to prevent the ReferenceError.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f16eb2a8603
Convert about:support to Fluent. r=Gijs,flod
https://hg.mozilla.org/integration/autoland/rev/32fe57d45d1b
Use data-l10n-id instead of formatValue where the change is relatively straightforward. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/c68ba2c62949
Test fixes. r=flod,Gijs

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/3f16eb2a8603
https://hg.mozilla.org/mozilla-central/rev/32fe57d45d1b
https://hg.mozilla.org/mozilla-central/rev/c68ba2c62949

Comment 13

[Jan Henning [:JanH]]

This broke about:support on Android - all localised text is now missing.