Last Comment Bug 679324 - remove the update channel reference from the about page when the user has no permissions to update anyway
: remove the update channel reference from the about page when the user has no ...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 9
Assigned To: Landry Breuil (:gaston)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-16 05:14 PDT by Wolfgang Rosenauer [:wolfiR]
Modified: 2011-08-24 17:28 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Dont show the update channel if the updater is disabled (1.42 KB, patch)
2011-08-17 05:02 PDT, Landry Breuil (:gaston)
margaret.leibovic: review-
kairo: feedback+
Details | Diff | Review
Dont show the update channel if the updater is disabled (2.33 KB, patch)
2011-08-22 14:28 PDT, Landry Breuil (:gaston)
margaret.leibovic: review+
Details | Diff | Review

Description Wolfgang Rosenauer [:wolfiR] 2011-08-16 05:14:25 PDT
Remove the update channel reference from the about page when the user has no permissions to update anyway.

This information is meaningless for people not having update rights or even the updater code shipped in Firefox builds.
Comment 1 Mike Hommey [:glandium] 2011-08-16 05:26:49 PDT
I think there are several things here:
- It's absolutely useless when the update channel is "default"
- It's informative but useless when there is not updater (--disable-updater)
- It's informative but useless when the user doesn't have the necessary rights to upgrade

I also think, but it's more of a long term thing, that distros could have it work through a package manager API or something like that.
Comment 2 Wolfgang Rosenauer [:wolfiR] 2011-08-16 05:55:48 PDT
(In reply to Mike Hommey [:glandium] from comment #1)
> I think there are several things here:
> - It's absolutely useless when the update channel is "default"
> - It's informative but useless when there is not updater (--disable-updater)

I don't think it's informative. It's a build time information and if not shipping with updater the compiled information is probably wrong anyway.

> - It's informative but useless when the user doesn't have the necessary
> rights to upgrade

If --enable-update then it might be informative because someone else on the system could have the needed permissions and the update channel is of interest.
 
> I also think, but it's more of a long term thing, that distros could have it
> work through a package manager API or something like that.

The information about the "update channel"?
My update channels are
mozilla, mozilla:beta, mozilla:alpha
currently and they are just defined by the configured repositories.
While I'm not against an idea like the above I do not see that something like that will happen just for mozilla.
Comment 3 Mike Hommey [:glandium] 2011-08-16 06:09:42 PDT
(In reply to Wolfgang Rosenauer [:wolfiR] from comment #2)
> (In reply to Mike Hommey [:glandium] from comment #1)
> > I think there are several things here:
> > - It's absolutely useless when the update channel is "default"
> > - It's informative but useless when there is not updater (--disable-updater)
> 
> I don't think it's informative. It's a build time information and if not
> shipping with updater the compiled information is probably wrong anyway.

If you're using the "aurora" or "beta" update channels, and use the mozilla update url, i'd expect that message to tell you if you're not using the last version, which is informative. I don't know if that's true, though.
Comment 4 Wolfgang Rosenauer [:wolfiR] 2011-08-16 06:11:33 PDT
Hmm, that might be a reason to show it but I'm pretty sure it doesn't behave that way at the moment.
Comment 5 Landry Breuil (:gaston) 2011-08-16 21:04:12 PDT
I realized the same thing.. as there's already an #ifdef MOZ_UPDATER in browser/base/content/aboutDialog.xul, the 'You are on the xxx update channel' could be included within it. I think it is misleading to say that if the update channel is provided by the os itself, nothing should be shown, or it could say smth along the lines of 'You are on your operating system package manager update channel'.
Comment 6 Robert Kaiser (not working on stability any more) 2011-08-17 04:48:08 PDT
Landry, the Mozilla code doesn't know if your OS provides updates for this build or not, so we can't really tell the user that, it would be wrong as well (if he's building himself, he gets the exact same "default" channel configuration).

But yes, this probably should be included in another #ifdef MOZ_UPDATER and just not be shown at all.
Comment 7 Landry Breuil (:gaston) 2011-08-17 05:01:04 PDT
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #6)
> Landry, the Mozilla code doesn't know if your OS provides updates for this
> build or not, so we can't really tell the user that, it would be wrong as
> well (if he's building himself, he gets the exact same "default" channel
> configuration).

Ah, you're right. That's a different issue..

> But yes, this probably should be included in another #ifdef MOZ_UPDATER and
> just not be shown at all.

That would make perfectly sense to me. How about the attached (totally untested) patch ?
Comment 8 Landry Breuil (:gaston) 2011-08-17 05:02:40 PDT
Created attachment 553731 [details] [diff] [review]
Dont show the update channel if the updater is disabled

Maybe we want to deal with all the issues at once, but knowing if the user has necessary rights to update the app is another thing.... as for not showing it too if its 'default'.
Comment 9 Robert Kaiser (not working on stability any more) 2011-08-20 11:51:44 PDT
Comment on attachment 553731 [details] [diff] [review]
Dont show the update channel if the updater is disabled

From my POV, this looks good, but I'm not entitled to doing reviews in that area, forwarding that to Margaret, given she also did the last review affecting this part of code. :)
Comment 10 :Margaret Leibovic 2011-08-22 13:33:29 PDT
Comment on attachment 553731 [details] [diff] [review]
Dont show the update channel if the updater is disabled

This looks like the right approach. However, it will cause a JS error, since we look for the currentChannel label here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/aboutDialog.js#92.

I think you should just move that block of code that sets the label value up a few lines to put it inside the #ifdef MOZ_UPDATER block that's currently above it. With that change, this should be good!
Comment 11 Landry Breuil (:gaston) 2011-08-22 14:28:15 PDT
Created attachment 554967 [details] [diff] [review]
Dont show the update channel if the updater is disabled

Something like this should address that issue.. i've checked, and defaults/channelLabel are not used elsewhere in the file, so move the whole block to the #ifdef.
Comment 12 Ed Morley [:emorley] 2011-08-24 07:58:35 PDT
Fixed missing bug number from commit message; landed on inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f2bc0f933661

Thanks Landry :-)
Comment 13 Matt Brubeck (:mbrubeck) 2011-08-24 17:28:47 PDT
http://hg.mozilla.org/mozilla-central/rev/f2bc0f933661

Note You need to log in before you can comment on or make changes to this bug.