1516799 - About:support is broken on android

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[csheany]

User Agent: Mozilla/5.0 (Android 7.1.1; Tablet; rv:66.0) Gecko/66.0 Firefox/66.0

Steps to reproduce:

1. Open about:support


Actual results:

Elements are missing


Expected results:

To be determined

Comment 1

[csheany]

Attachment: Screenshot_20181229-144546.jpg

Comment 2

[Francesco Lodolo [:flod]]

I thought it could be an issue with missing fallback to English, but it happens also in Italian, where I've already done the migration (and it works correctly on Desktop).

I've looked again at
https://hg.mozilla.org/mozilla-central/rev/c68ba2c62949

And I can't spot any glaring issue. 

But in my build, about:about is also not localized (it's in English, no strings missing), so there might be a larger problem with FTL and mobile builds.

Comment 3

[Francesco Lodolo [:flod]]

I'm completely lost, since I don't know much about the build process or l10n registry.

en-US files:
resource:///localization/en-US/toolkit/en-US/toolkit has 4 folders, the /about folder has 10 files (including aboutSupport.ftl). So we should at least fall back, unless we're looking in a different path.

resource:///localization/it/toolkit/ has only one file (aboutAbout.ftl).

The structure of resource:///localization/ is also completely different from what I see on desktop.

Comment 4

[Axel Hecht [:Pike]]

Can't figure out how to do a local build in a descent amount of time, but yes, the apk contents are puzzling.

Comment 5

[Francesco Lodolo [:flod]]

I've double checked Beta and Release, and I see the same unlocalized about:about and resource:/// structure, so it's not a regression (or, at least, not recent).

Comment 6

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

Chenxia, do you know about packaging for Android builds?

Comment 7

[:Gijs (he/him)]

So to be clear, what's broken here? My en-US beta build has working strings on about:about, so it seems to me fluent is not completely broken. Is non-en-US broken? Something else?

Comment 8

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

When I visit "resource:///localization/en-US/toolkit/en-US/toolkit" on my Nightly Android build, I get a "file not found" error.

However, "resource:///localization/en-US/toolkit/" shows the four folders that comment #3 is referring to. Inside of the "about" folder there is an "aboutSupport.ftl" file.

Comment 9

[Francesco Lodolo [:flod]]

(In reply to :Gijs (he/him) from comment #7)
> So to be clear, what's broken here? My en-US beta build has working strings
> on about:about, so it seems to me fluent is not completely broken. Is
> non-en-US broken? Something else?

* Build: we're not packaging all localized files. In the locale path there's only one file (aboutAbout.ftl), and it's in the wrong path as far as I can tell (it shouldn't be in the root). No trace of aboutSupport.ftl
* about:support it's not even loading en-US strings as fallback (like it happens for about:about). Not sure if there's something in the code causing that.

(In reply to (away Dec 24-26, Dec 31-Jan 1) Jared Wein [:jaws] (Regression Engineering Owner for 65) (please needinfo? me) from comment #8)
> When I visit "resource:///localization/en-US/toolkit/en-US/toolkit" on my
> Nightly Android build, I get a "file not found" error.

From my tests yesterday, you need the trailing slash.

Comment 10

[csheany]

Also, Bug 1493531 and Bug 1512729

Comment 11

[Francesco Lodolo [:flod]]

(In reply to csheany from comment #10)
> Also, Bug 1493531 and Bug 1512729

Let's not go off-topic. This bug is about the page being broken after the migration to Fluent, and that has nothing to do with changing features or aspect of the page.

Comment 12

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #9)
> * Build: we're not packaging all localized files. In the locale path there's
> only one file (aboutAbout.ftl), and it's in the wrong path as far as I can
> tell (it shouldn't be in the root). No trace of aboutSupport.ftl

Correction: in Today's build from Play Store, aboutSupport.ftl is packaged together with aboutAbout.ftl

Comment 13

[Francesco Lodolo [:flod]]

Trying also to NI zibi, in case he has ideas on what's going on.

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

The reason this doesn't work is because we reference brand.ftl which doesn't exist in Android:

```
+    <link rel="localization" href="branding/brand.ftl"/>
+    <link rel="localization" href="toolkit/about/aboutSupport.ftl"/>
```

aboutSupport.ftl gets properly loaded, but `branding/brand.ftl` only exists on desktop and gets packaged into browser's `omni.ja` (and in result FileSource).

Since Fennec only has a single omni.ja, I'm not sure how to approach it - the easiest way would be to ignore the fact that the GRE omni.ja is supposed to be app-independent, and just package https://searchfox.org/mozilla-central/source/mobile/android/branding/*/locales/en-US/brand.dtd into it just like we do for browser's brand.ftl:
https://searchfox.org/mozilla-central/source/browser/branding/official/locales/jar.mn
https://searchfox.org/mozilla-central/source/mobile/android/branding/official/locales/jar.mn

Comment 15

[:Gijs (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #14)
> The reason this doesn't work is because we reference brand.ftl which doesn't
> exist in Android:
> 
> ```
> +    <link rel="localization" href="branding/brand.ftl"/>
> +    <link rel="localization" href="toolkit/about/aboutSupport.ftl"/>
> ```
> 
> aboutSupport.ftl gets properly loaded, but `branding/brand.ftl` only exists
> on desktop and gets packaged into browser's `omni.ja` (and in result
> FileSource).
> 
> Since Fennec only has a single omni.ja, I'm not sure how to approach it -
> the easiest way would be to ignore the fact that the GRE omni.ja is supposed
> to be app-independent, and just package
> https://searchfox.org/mozilla-central/source/mobile/android/branding/*/
> locales/en-US/brand.dtd into it just like we do for browser's brand.ftl:
> https://searchfox.org/mozilla-central/source/browser/branding/official/
> locales/jar.mn
> https://searchfox.org/mozilla-central/source/mobile/android/branding/
> official/locales/jar.mn

FWIW, this doesn't seem to be sufficient. We also need to add a FileSource that actually maps to this part of omni.ja, which feels pretty yucky. From what I saw of L10nRegistry internals when looking at this, it seems there may be runtime overheads to this because we attempt to find any ftl file in any registered file source, which will involve lots of poking at bits of omni.ja that have nothing except the branding files. Presumably there is a better solution to this, but none was pointed out here...

Perhaps the alternative would be to find a way to package the branding file in the expected part of the existing "toolkit" FileSource.

Comment 16

[:Gijs (he/him)]

Attachment: Bug 1516799 - add fluent branding on fennec, r?zbraniecki!,Pike!

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Does this definitely affect beta and release?

Comment 18

[Francesco Lodolo [:flod]]

(In reply to Liz Henry (:lizzard) (PTO Dec 28) from comment #17)
> Does this definitely affect beta and release?

No, it only affects Nightly and mobile (page works fine on desktop).

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c2796200711
add fluent branding on fennec, r=zbraniecki

Comment 21

[Axel Hecht [:Pike]]

Gijs, thanks for taking a stab at this, the landing looks OK. Let's see what the landing on central brings on the next nightly.

Clearing the pending needinfos.

Comment 22

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6c2796200711

Comment 23

[Francesco Lodolo [:flod]]

Latest Nightly (20190104093221) from Play Store has text, but it's in English. Filed bug 1517919 for that part.

Comment 24

[Stefan Deiac]

Verified as fixed on the latest version of Nightly 66.0a1 (2019-01-06) with Nexus 6P (Android 8.1.0). Due to that and the comment 23, I'll remove the qe-verify flag, thanks.