644517 - Implement channel selector UI for updates

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Alex Limi (:limi) — Firefox UX Team]

For the upcoming versions of Firefox, we want users to be able to opt in to experimental and beta channels if they want to, via the same dialog that holds the version information (currently the About window).

The user should be able to move freely between experimental, beta and final release update channels, and we should give a mechanism to restart when you change the channel.

Some explicit non-goals:

* We don't allow people to switch to nightly builds from this dialog, since they are for developers, and should be a separate download (and arguably shouldn't even use the same profile, but that's a separate issue).

* We don't allow people to switch to project branches using this selector. If someone wants to write an add-on that does this, that's fine — but we're not going to do it in the core implementation.

Comment 1

[Robert Strong (they/them - no direct email)]

Depending on where the ui is placed will determine the component but it definitely won't be in the installer so moving to General until that is figured out.

Comment 2

[Alex Limi (:limi) — Firefox UX Team]

Attachment: Channel selector in about window, sketch

First draft of the channel selector UI in About window.

Comment 3

[Alex Limi (:limi) — Firefox UX Team]

Other notes from UX session:

- Will Test Pilot be in the Experimental and Beta builds always? (I assume yes)
- Tone down the channel wording, maybe have a "recommended" on the stable channel
- Possibly add some additional explanation after you have clicked "Change" that explain what is supposed to happen

Comment 4

[:Margaret Leibovic]

I'm taking this. I'll start building the front-end from Limi's mockup, and I can work with Rob Strong to hook it up to the back-end pieces when they're ready.

Comment 5

[:Margaret Leibovic]

Attachment: wip

This patch gets the xul/css pieces of this feature working. I made a stub function to hook into the back-end, and hopefully we can just flesh that out once Rob's work is ready.

I just tested this on OSX with my default system settings, but I'll test it on other platforms and with different system settings (like large font) to make sure it looks okay.

Comment 6

[Axel Hecht [:Pike]]

As I've mentioned in .planning just now, the availability of a channel will probably depend on locale. Not sure how to hook that up.

Comment 7

[Robert Strong (they/them - no direct email)]

The simplest way to handle this I think would be if the ui handled the no update available condition with an appropriate message when trying to change channels. This *should* only happen when trying to go to less stable channels and never prevent going to stable after the first release that contains channel changing.

Limi / Margaret, what do you think and if this is acceptable can you work out the details?

Comment 8

[Axel Hecht [:Pike]]

I expect that new locales will be available in beta, but not in release, until they're shaken out. The count of locales across channels will be some kind of bell curve. Low on central, really darn high on mercury, really high on beta, pretty really high on release. Wow, that's soooo not English, but you get the idea.

Comment 9

[Robert Strong (they/them - no direct email)]

All of which should be fine with the approach outlined in comment #7

Comment 10

[Axel Hecht [:Pike]]

Right, a useful error report works either way. Can we tell apart a plain network error apart from an unsupported channel?

I would hope that at some point we can get the available channels upfront, so that people don't get frustrated on us. Having folks trying to get from beta to release every other day while we unbeta a locale sounds like bad mojo.

Comment 11

[Robert Strong (they/them - no direct email)]

(In reply to comment #10)
> Right, a useful error report works either way. Can we tell apart a plain
> network error apart from an unsupported channel?
We can currently tell a network error from an empty update (e.g. changing to that channel is not available at this time). The empty update could include unsupported channel info.

> I would hope that at some point we can get the available channels upfront, so
> that people don't get frustrated on us. Having folks trying to get from beta to
> release every other day while we unbeta a locale sounds like bad mojo.
Might be best to only support specific channels for locales that opt-in for now and not show the ui for the locales that haven't opted-in.

Comment 12

[:Margaret Leibovic]

Attachment: wip v2

This still needs:
-Final channel description strings (I just made things up)
-Correct connection with backend (I just made a temporary guess of what it might look like)
-Solution to "some locales won't have updates for all channels" problem
-Localization notes

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 523640 [details] [diff] [review]
wip v2

>+#contentDeck[selectedIndex="0"] > #detailsBox,
>+#contentDeck[selectedIndex="1"] > #channelSelector,
>+#channelDescriptionDeck[selectedIndex="0"] > #defaultDescription,
>+#channelDescriptionDeck[selectedIndex="1"] > #betaDescription,
>+#channelDescriptionDeck[selectedIndex="2"] > #experimentalDescription {
>+  opacity: 1;
>+}

this is a no-op

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Margaret, have you discussed with Rob Strong how we are going to store the channel pref ? At the moment we always read the default pref branch and ignore any value set in the profile:
 http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#586
Crash reporter does this too, and possibly other places like blocklist.

If we want to stick with channel-prefs.js then Mossop has experience changing that file in old versions of Nightly Tester Tools

Comment 15

[:Margaret Leibovic]

Comment on attachment 523640 [details] [diff] [review]
wip v2

(In reply to comment #14)
> Margaret, have you discussed with Rob Strong how we are going to store the
> channel pref ?

I was just flipping the profile pref for now to test the way to UI acts when switching channels. I also just guessed the app updater call we'll be making when we switch channels. This will be fleshed out more when the backend is ready for end to end testing.

Comment 16

[Robert Strong (they/them - no direct email)]

(In reply to comment #14)
> Margaret, have you discussed with Rob Strong how we are going to store the
> channel pref ? At the moment we always read the default pref branch and ignore
> any value set in the profile:
> 
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#586
> Crash reporter does this too, and possibly other places like blocklist.
> 
> If we want to stick with channel-prefs.js then Mossop has experience changing
> that file in old versions of Nightly Tester Tools
We will stick with channel-prefs.js for testing and replace it when the channel actually changes (see bug 386760 for how this is accomplished).

A nice side affect of doing it this way is that the channel will be accurate in that it will reflect the channel that is being used.

I very briefly spoke with morgamic about needing the ability to switch channels and providing an update snippet to the latest update for the channels involved. What would be simplest would be to add a param along the lines of changeTo=channel_name and have aus serve up a snippet with the latest update for that channel with only a complete update. I could make the update service ignore any partial info though that is adding complexity to the client.

Comment 17

[Robert Strong (they/them - no direct email)]

btw: if just swapping out the channel name is enough for aus I could just set the default pref for the channel instead and it will become permanent after the update completes.

Comment 18

[:Margaret Leibovic]

Attachment: patch

This just needs channel description strings from marketing.

Comment 19

[Axel Hecht [:Pike]]

A comment on channel names, I'm not sure if we should translate Beta, and can even see arguments for not translating "Release". Mostly about not confusing support folks when asking users for what channel they're on. I'm not sure that Aurora as a name is *all* *that* different from the other channel names in whether we should or not should allow localized variants of those names.

Also, is there a suggestion for RTL where at least the Aurora channel is gonna switch text direction.

Comment 20

[:Margaret Leibovic]

Attachment: patch v2

This addresses Gavin's real-life comments.

Comment 21

[Axel Hecht [:Pike]]

What were those real-life comments, if I may?

Comment 22

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 524448 [details] [diff] [review]
patch v2

>diff --git a/browser/base/content/aboutDialog.js b/browser/base/content/aboutDialog.js

>+var gChannelSelector = {
>+  init: function() {

>+      this.channelValue = Services.prefs.getCharPref("app.update.channel");

I suppose we should actually be getting this from the default pref branch:
let defaults = Services.prefs.getDefaultBranch("");
this.channelValue = defaults.getCharPref("app.update.channel");

Thinking about this a little more, I think we need to handle the channel being something we don't recognize (e.g. "nightly", "default", or some other arbitrary string) by just hiding this switching UI.

>diff --git a/browser/base/content/aboutDialog.xul b/browser/base/content/aboutDialog.xul

>+                  <menuitem id="defaultMenuitem" label="Release" value="default"/>

I think this value should be "release", not "default".

>+            <!-- For CSS transition effect, set an initial value for selectedIndex in case
>+                 it does not get set by gChannelSelector.setChannelDescription() -->

I'd change this to "Make sure the selectedIndex attribute is always set so that the CSS selectors for transitions work". And perhaps I'd move it to the first instance of this (the other deck).

>diff --git a/browser/locales/en-US/chrome/browser/aboutDialog.dtd b/browser/locales/en-US/chrome/browser/aboutDialog.dtd

>+<!ENTITY channel.description.start  "You are currently on the ">
>+<!ENTITY channel.description.end    " update channel. ">

>+<!ENTITY channel.selector.start          "Switch to the">
>+<!ENTITY channel.selector.end            "update channel.">

Do things work OK if either/both of these strings are very long?

>+<!ENTITY channel.stable.description        "This channel is stable and awesome and won't crash and will make you very happy.">
>+<!ENTITY channel.beta.description          "This channel is almost perfect but still needs testing to make sure it's okay to ship to everyone.">
>+<!ENTITY channel.experimental.description  "This channel might be broken and make you sad sometimes but you'll be on the cutting edge and your nerd friends will be jealous.">

I thought Laura had new strings for this?

Comment 23

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #21)
> What were those real-life comments, if I may?

I suggested the changes from the interdiff:
https://bugzilla.mozilla.org/attachment.cgi?oldid=524283&action=interdiff&newid=524448&headers=1

(I concurred with you re: not localizing the channel names)

Comment 24

[:Margaret Leibovic]

Attachment: patch v3

> >+<!ENTITY channel.selector.start          "Switch to the">
> >+<!ENTITY channel.selector.end            "update channel.">
> 
> Do things work OK if either/both of these strings are very long?

When these are too long, the window gets stretched to be wider. There is still room for localizers to use longer strings, so hopefully there is enough room?

Comment 25

[:Margaret Leibovic]

Attachment: patch v3 (corrected)

I forgot to update some selectors in my CSS. Fixed now.

Comment 26

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 524636 [details] [diff] [review]
patch v3 (corrected)

r=me with the non-loopy validChannels check we discussed IRL

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I have talked with Margaret on Monday about the addition of automated tests, and it's something she wants to take care of once it has been landed. That's definitely the higher priority at the moment. Once we have those tests we will investigate necessary tests with Mozmill.

Comment 28

[:Margaret Leibovic]

Attachment: patch for check-in

Comment 29

[:Margaret Leibovic]

Landed. This won't be visible on nightly builds because we will only show this UI for release/beta/aurora update channels. However, you can test what it looks like by setting app.update.desiredChannel to one of those channels.

http://hg.mozilla.org/mozilla-central/rev/ce7276f42938

Comment 30

[Francesco Lodolo [:flod]]

Not sure if that's a behavior by design or a problem, I thought it was better to ask here before opening an unnecessary bug.

I'm doing some local tests with a nightly build and I realized that in Italian I don't need the second part of the channel selector string (the one displayed after the drop-down list), so I left channel.selector.end empty. Strangely enough the result is that the English string is displayed, so I had to use a blank space for that string.

Comment 31

[Francesco Lodolo [:flod]]

Please ignore my last comment and sorry for the spam. After checking the patch I realized that the string is not processed by JavaScript or anything, so I double checked and found that the problem was in the tool I use to translate (empty string=automatically export the original en-US string).

Comment 32

[Gabriela [:gaby2300]]

Verified fixed in

Mozilla/5.0 (Windows NT 6.1; rv:5.0a2) Gecko/20110506 Firefox/5.0a2

Comment 33

[Kevin Brosnan [Ex-Mozilla]]

Not turned on for Nightly builds. 

Verified Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0b1) Gecko/20100101 Firefox/5.0