Closed Bug 644517 Opened 14 years ago Closed 14 years ago

Implement channel selector UI for updates

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 5

People

(Reporter: limi, Assigned: Margaret)

References

Details

(Whiteboard: [channel-switcher][bugday-2011-05-06])

Attachments

(2 files, 6 obsolete files)

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.
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.
Component: Installer → General
QA Contact: installer → general
First draft of the channel selector UI in About window.
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
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.
Assignee: nobody → margaret.leibovic
Status: NEW → ASSIGNED
Attached patch wip (obsolete) (deleted) — Splinter Review
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.
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.
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?
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.
All of which should be fine with the approach outlined in comment #7
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.
(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.
Attached patch wip v2 (obsolete) (deleted) — Splinter Review
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
Attachment #522809 - Attachment is obsolete: true
Attachment #523640 - Flags: feedback?(gavin.sharp)
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
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 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.
Attachment #523640 - Flags: feedback?(robert.bugzilla)
(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.
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.
Attached patch patch (obsolete) (deleted) — Splinter Review
This just needs channel description strings from marketing.
Attachment #523640 - Attachment is obsolete: true
Attachment #523640 - Flags: feedback?(robert.bugzilla)
Attachment #523640 - Flags: feedback?(gavin.sharp)
Attachment #524283 - Flags: review?(gavin.sharp)
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.
Attached patch patch v2 (obsolete) (deleted) — Splinter Review
This addresses Gavin's real-life comments.
Attachment #524283 - Attachment is obsolete: true
Attachment #524283 - Flags: review?(gavin.sharp)
Attachment #524448 - Flags: review?(gavin.sharp)
What were those real-life comments, if I may?
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?
(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)
Attached patch patch v3 (obsolete) (deleted) — Splinter Review
> >+<!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?
Attachment #524448 - Attachment is obsolete: true
Attachment #524448 - Flags: review?(gavin.sharp)
Attachment #524509 - Flags: review?(gavin.sharp)
Attached patch patch v3 (corrected) (obsolete) (deleted) — Splinter Review
I forgot to update some selectors in my CSS. Fixed now.
Attachment #524509 - Attachment is obsolete: true
Attachment #524509 - Flags: review?(gavin.sharp)
Attachment #524636 - Flags: review?(gavin.sharp)
Comment on attachment 524636 [details] [diff] [review] patch v3 (corrected) r=me with the non-loopy validChannels check we discussed IRL
Attachment #524636 - Flags: review?(gavin.sharp) → review+
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.
Flags: in-testsuite?
Attached patch patch for check-in (deleted) — Splinter Review
Attachment #524636 - Attachment is obsolete: true
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox4.2
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.
Target Milestone: Firefox5 → Firefox 3
Target Milestone: Firefox 3 → Firefox 5
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).
Whiteboard: [channel-switcher]
Verified fixed in Mozilla/5.0 (Windows NT 6.1; rv:5.0a2) Gecko/20110506 Firefox/5.0a2
Not turned on for Nightly builds. Verified Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0b1) Gecko/20100101 Firefox/5.0
Status: RESOLVED → VERIFIED
Whiteboard: [channel-switcher] → [channel-switcher][bugday-2011-05-06]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: