Pref off the langpack download UI except for release and beta

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
7 months ago
17 days ago

People

(Reporter: mstriemer, Assigned: mstriemer)

Tracking

(Blocks 1 bug)

unspecified
Firefox 65
Points:
---

Firefox Tracking Flags

(firefox64+ fixed, firefox65 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

7 months ago
Currently the entire UI is behind the same pref, but the search and download will only work on release for now.

The AMO API currently only provides langpack info for the release channel. Once the API is updated to support dev-edition/beta we can open this UI up to those channels.

Comment 2

6 months ago
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f07a9906ade4
Pref off downloading langpacks outside of release r=jaws

Comment 3

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f07a9906ade4
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
(Assignee)

Comment 4

6 months ago
Comment on attachment 9017684 [details]
Bug 1493711 - Pref off downloading langpacks outside of release r?jaws

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: None

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This adds a second pref for the new multilingual UI that is different on release. These changes are needed in 64 for testing on 64 release to confirm things are okay for being enable in 65.

String changes made/needed: None
Attachment #9017684 - Flags: approval-mozilla-beta?
(In reply to Mark Striemer [:mstriemer] from comment #4)
> User impact if declined: None

No user impact if we don't take this?

Also, AFAIK, #ifdef RELEASE isn't a thing. Have you tested that this change actually works like you expect?
Flags: needinfo?(mstriemer)
(Assignee)

Comment 6

6 months ago
It sets the pref as I expected on Nightly, but that could just be since RELEASE in undefined. It was used in a couple [1] other places, but I don't see anything defining it so you could be right that it won't work.

> No user impact if we don't take this?

This just wraps some release specific code in a pref, and that code is already preffed off. Testing for this needs to happen in release 64 but we don't plan on enabling it until 65. This portion of the code pulls data from AMO and that endpoint only provides data for release.

So basically we want to uplift this so we can test it a release early.
Flags: needinfo?(mstriemer)
I'm about 99% positive it won't work. We have the RELEASE_OR_BETA ifdef which gets set based on version number, but we don't have any release-specific ifdefs in our codebase. About the only thing you could do would be to sniff based on update channel.
Per IRC discussion with Jared, I'm reopening the bug for a follow-up patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 65 → ---
(Assignee)

Comment 9

6 months ago
It looks like we can probably use UpdateUtils and let the pref be an override to enable on Beta/Nightly.

  Services.prefs.getBoolPref("intl.multilingual.downloadEnabled")
    || UpdateUtils.getUpdateChannel(false) == "release"
In general, we try to avoid behavior changes which don't take effect until release because it makes QA difficult and can be brittle (like for downstream distros). Can we use EARLY_BETA_OR_EARLIER instead, which would get you through the midway point of the Beta cycle? Anyway, I'll leave for you and Jared to discuss further.
(Assignee)

Comment 11

6 months ago
Just had some discussions with Callek and mat (among others) and it looks like we should be able to get langpacks up on AMO for beta releases. This would allow us to pref this off for Nightly, but enable it everywhere else which sounds much much better.

We could hypothetically turn this on for EARLY_BETA_OR_EARLIER but it will never work on beta in the current state. You'll never get results from the server since it only gives results for versions that have been released.

Callek is going to try and get the langpacks sorted on AMO this week, then I should be able to update this to enable it on not-Nightly.
Comment on attachment 9017684 [details]
Bug 1493711 - Pref off downloading langpacks outside of release r?jaws

a- for this on beta64 based on the discussion in the bug.
Attachment #9017684 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Comment 14

6 months ago
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff6c293b7077
Enable langpack download on release and Beta r=jaws

Comment 15

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ff6c293b7077
Status: REOPENED → RESOLVED
Last Resolved: 6 months ago6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
(Assignee)

Updated

6 months ago
Summary: Pref off the langpack download UI except for release → Pref off the langpack download UI except for release and beta
(Assignee)

Comment 16

6 months ago
Comment on attachment 9022757 [details]
Bug 1493711 - Enable langpack download on release and Beta r?jaws

No QE needed since the related bugs will already be tested.

Both patches in this bug would need to be uplifted.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: A pref will need to be flipped to install language packs on release and beta

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Changes a pref on release and beta, but it is already behind a separate disabled pref.

String changes made/needed:
Attachment #9022757 - Flags: approval-mozilla-beta?
I'm slightly confused here.  I was under the impression that the next effect of these two patches is to not change anything on beta/release, but to disable langpack download on nightly.  Did I get that wrong?
Flags: needinfo?(mstriemer)
(Assignee)

Comment 18

6 months ago
This feature is currently being tested on Beta and will be tested on release as well. Technically this shouldn't make any difference to Beta or release, but the testing is happening now and it might be good to have this included while testing is happening.
Flags: needinfo?(mstriemer)
Comment on attachment 9017684 [details]
Bug 1493711 - Pref off downloading langpacks outside of release r?jaws

alright thanks for the explanation.  Let's get this in 64.0b8.
Attachment #9017684 - Flags: approval-mozilla-beta- → approval-mozilla-beta+
Attachment #9022757 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.