Closed Bug 642765 Opened 9 years ago Closed 9 years ago

Add ability to channel change to the client

Categories

(Toolkit :: Application Update, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

(Whiteboard: [channel-switcher])

Attachments

(2 files, 4 obsolete files)

The plan is to add a file that contains a list of all files in an update, remove all of those files for a complete update, and use the existing process to install the files from the complete mar file. Since Bug 386760 requires a new update manifest format I have added "type" to the update manifest in that bug and when type is "complete" remove operations will be added to the actions for all files listed in the file list.
btw: this bug is just about the updater... there will be additional bugs for creating the ui and for any changes needed to the update service component.
Attached patch patch rev1 (obsolete) — Splinter Review
The patch in bug 386760 takes care of the updater so this bug will be used for the changes to the client.

The patch has very minimal tests and I'll finish the tests hopefully later today. I was able to manually test a channel change using this patch along with the patches in bug 386760 and with the patch in bug 644517 which has already landed.
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Attachment #525050 - Flags: review?(dtownsend)
Summary: Add ability to downgrade to the updater → Add ability to channel change to the client
note: I still need to force downloading the complete mar if there is a channel change instead of relying on the update snippet to only contain a complete.
Comment on attachment 525050 [details] [diff] [review]
patch rev1

>diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js
>--- a/toolkit/mozapps/update/nsUpdateService.js
>+++ b/toolkit/mozapps/update/nsUpdateService.js
>...
>@@ -100,16 +101,17 @@ const KEY_GRED            = "GreD";
> #define USE_UPDROOT
> #endif
> 
> #ifdef USE_UPDROOT
> const KEY_UPDROOT         = "UpdRootD";
> #endif
> 
> const DIR_UPDATES         = "updates";
>+const FILE_CHANNELCHANGE  = "channelchange";
should be the following and I fixed this locally.
const FILE_CHANNELCHANGE  = "changechannel";
Attached patch patch rev2 (obsolete) — Splinter Review
Fixed the typo and cleared the pref on success... still need to add more tests.
Attachment #525050 - Attachment is obsolete: true
Attachment #525050 - Flags: review?(dtownsend)
Attachment #525056 - Flags: review?(dtownsend)
Comment on attachment 525056 [details] [diff] [review]
patch rev2

Looks pretty good. I assume the channel change file gets deleted when the update is applied. One though, why not just write "*" or "null" to the version file instead of having to bother with the channel change file at all? Seems like the less magic files we have to manage the better.
Attachment #525056 - Flags: review?(dtownsend) → review+
nit: the two places where you have:

desiredChannel ? desiredChannel : getUpdateChannel()

you could instead just have:

getDesiredChannel() || getUpdateChannel()
Attached patch patch updated to comments (obsolete) — Splinter Review
Mossop, I went with a new file so its existence could just be checked by the updater binary which also needs to know when there is a channel change vs. adding code to read the file. It does get deleted after a successful update.

Thanks Gavin... I completely spaced.
Attachment #525056 - Attachment is obsolete: true
Attachment #525123 - Flags: review+
forgot to remove the redundant calls
Attachment #525123 - Attachment is obsolete: true
Attachment #525155 - Flags: review+
Attachment #525156 - Attachment is patch: true
Attachment #525156 - Attachment mime type: application/octet-stream → text/plain
Attachment #525156 - Flags: review?(dtownsend) → review+
Changed
const FILE_CHANNELCHANGE  = "changechannel";

to

const FILE_CHANNELCHANGE  = "channelchange";

due to review comments in bug 386760
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/30d29e1da9e3
http://hg.mozilla.org/mozilla-central/rev/b4dcf0feefd1

There are a couple of tests already added but I have a couple of more I would like to add.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite?
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla5
Whiteboard: [channel-switcher]
Does this mean we can now fix bug 657789 ?
No, channel switching functionality was removed a long time ago. Also, this bug doesn't help with bug 657789 since bug 657789 is about *us* moving people to different channels and this is about us giving the *user* the ability to switch channels.
does it make sense for *us* to be switchging people around on channels, with out the user being able to control this as well?  seems like that is setting us up for many frustrated users with we switch them, and its difficult for them to change back to what they might be more comfortable with.
Let's take that discussion over to bug 657789?
(In reply to chris hofmann from comment #18)
> does it make sense for *us* to be switchging people around on channels, with
> out the user being able to control this as well?  seems like that is setting
> us up for many frustrated users with we switch them, and its difficult for
> them to change back to what they might be more comfortable with.
Seems to me that just changing this without the user initiating the change is the real issue though I can see conflating this issue to include the ability for the user to change the channels can help with rationalizing why it would be ok to change the user's channel without them initiating.

Whatever the case, channel switching has several issues that were agreed to initially by all decision making groups (Drivers, Engineering, UX, Marketing) with specific workarounds that were also agreed upon by these groups. After the implementation it was decided that these conditions were not acceptable so the functionality was removed. The issues are due to platform limitations that can not be worked around as long as the workarounds that were initially agreed upon are not acceptable.
The problem we need to solve is:

There's no nightly build for Assamese.

For the users on there, we can either give them the channel they want and change their language, or we give them the language they want and change their channel.

I'd rather keep the testers on the localizations, thus I favor a channel switch.
Which is what bug 657789 is about... specifically it is about us moving a user to a different channel. This bug is about providing users with the ability to switch channels.
You need to log in before you can comment on or make changes to this bug.