Broken down from bug 1062345. The mozilla.org page that offers to reset Firefox needs to be able to determine the user's Firefox version. We should add an action to UITour that allows that.

Oh, I think instead of adding a new UITour action, Gavin was saying in bug 1062345 comment 7 (re: the "is sync enabled?" thing) that we should add a configuration recognized by getConfiguration, similar to the "sync" one there that has sends a boolean "setup": http://mxr.mozilla.org/mozilla-central/source/browser/modules/UITour.jsm?rev=08ba579779b1#1127

(In reply to Drew Willcoxon :adw from comment #1) > Oh, I think instead of adding a new UITour action, Gavin was saying in bug > 1062345 comment 7 (re: the "is sync enabled?" thing) that we should add a > configuration recognized by getConfiguration, similar to the "sync" one > there that has sends a boolean "setup": I agree with this FWIW.

Comment on attachment 8491158 [details] [diff] [review] Determine user's Firefox version from UITour Review of attachment 8491158 [details] [diff] [review]: ----------------------------------------------------------------- Looking good but I'll take another look. ::: browser/modules/UITour.jsm @@ +24,5 @@ > "resource:///modules/BrowserUITelemetry.jsm"); > +XPCOMUtils.defineLazyGetter(this, "appInfo", () => { > + return Cc["@mozilla.org/xre/app-info;1"] > + .getService(Ci.nsIXULAppInfo) > + .QueryInterface(Ci.nsIXULRuntime); I think you can just use Services.appinfo or is this doing something smarter? @@ +1139,5 @@ > setup: Services.prefs.prefHasUserValue("services.sync.username"), > }); > break; > + case "version": > + this.sendPageCallback(aContentDocument, aCallbackID, { Before seeing this patch I was writing: Perhaps "appinfo" could be the configuration and it would include various bits of info from Services.appinfo as needed. @@ +1140,5 @@ > }); > break; > + case "version": > + this.sendPageCallback(aContentDocument, aCallbackID, { > + version: appInfo.version I think it would be useful to include some other properties such as ID (to be sure it's Firefox), defaultUpdateChannel, distributionID, isOfficialBranding, isReleaseBuild in case we want to prevent offering a reset for unofficial, non-release, and/or partner builds. e.g. Suppose I'm running a partner build with a default search engine other than Google. If I go to mozilla.org to download vanilla Firefox, I should be able to easily download it instead of being redirect to do a reset which may leave me with partner customizations that I don't want. ::: browser/modules/test/browser_UITour.js @@ +9,5 @@ > > Components.utils.import("resource:///modules/UITour.jsm"); > +let appInfo = Components.classes["@mozilla.org/xre/app-info;1"] > + .getService(Ci.nsIXULAppInfo) > + .QueryInterface(Ci.nsIXULRuntime); Use Services.appinfo inline

Might as well include vendor and name as well to make sure we can distinguish third-party builds like Frontmotion Firefox which may use the same ID for add-on compat (I'm not sure).

Comment on attachment 8491208 [details] [diff] [review] Determine user's Firefox version from UITour Review of attachment 8491208 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: browser/modules/test/browser_UITour.js @@ +269,5 @@ > + function callback(result) { > + let props = ["defaultUpdateChannel", "distributionID", "isOfficialBranding", > + "isReleaseBuild", "name", "vendor", "version"]; > + for (let property of props) { > + is(result[property], Services.appinfo[property], "Should have the same " + property + " property."); It would probably be a good idea to also make sure these aren't undefined to make sure we're not comparing undefined to undefined: ok(typeof(result[property]) !== undefined, "Check " + property + " isn't undefined."); ^ untested code

Good idea to check for that undefined :-)

Try run looks good: https://tbpl.mozilla.org/?tree=Try&rev=1480c4f5fa90

>diff --git a/browser/modules/UITour.jsm b/browser/modules/UITour.jsm >+ case "appinfo": >+ let props = ["defaultUpdateChannel", "distributionID", "isOfficialBranding", >+ "isReleaseBuild", "name", "vendor", "version"]; This is much broader than the "version" that's in this bug's summary. Do we really need to read all of this for this use case?

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #12) > This is much broader than the "version" that's in this bug's summary. Do we > really need to read all of this for this use case? See comment 4 and comment 5. The bug summary and commit message should have been changed. I just did a quick pass through Services.appinfo to see which properties would be relevant for the usage in bug 1027318. Looks like Alex forgot to include "ID".

(In reply to Matthew N. [:MattN] from comment #4) > I think it would be useful to include some other properties such as ID (to > be sure it's Firefox), defaultUpdateChannel, distributionID, > isOfficialBranding, isReleaseBuild in case we want to prevent offering a > reset for unofficial, non-release, and/or partner builds. The UITour mechanism only works with Firefox, so the ID seems unnecessary. Similarly, I don't see any need to filter by any of the other properties, so I don't think we should send them. If that changes later, we can revisit. > e.g. Suppose I'm running a partner build with a default search engine other > than Google. If I go to mozilla.org to download vanilla Firefox, I should be > able to easily download it instead of being redirect to do a reset which may > leave me with partner customizations that I don't want. There should be a way to "download anyways" regardless, for users who want that. I don't think we need complicated filtering here.

I filed bug 1070138 to revise this.

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #14) > (In reply to Matthew N. [:MattN] from comment #4) > > I think it would be useful to include some other properties such as ID (to > > be sure it's Firefox), defaultUpdateChannel, distributionID, > > isOfficialBranding, isReleaseBuild in case we want to prevent offering a > > reset for unofficial, non-release, and/or partner builds. > > The UITour mechanism only works with Firefox, so the ID seems unnecessary. > Similarly, I don't see any need to filter by any of the other properties, so > I don't think we should send them. If that changes later, we can revisit. It can work in any build of the browser app which isn't necessarily Firefox. We're trying to move faster and put more trust in mozilla.org recently so adding this to prevent bottlenecks later seemed worthwhile (c.f. your recent questions about UITour whitelisting and bug 1068401). I don't see how this hurts and why we would spend time removing the additional values. > > e.g. Suppose I'm running a partner build with a default search engine other > > than Google. If I go to mozilla.org to download vanilla Firefox, I should be > > able to easily download it instead of being redirect to do a reset which may > > leave me with partner customizations that I don't want. > > There should be a way to "download anyways" regardless, for users who want > that. I don't think we need complicated filtering here. At one point there was talk about not providing a download option. I'll reply to the rest in bug 1070138.

Hi, is it possible to uplift this (and Bug 1070138) to Firefox 34 Beta assuming the risk is low? As you might know, www.mozilla.org and SUMO want to utilize the new API for version check. The current implementation on www.mozilla.org has confused visitors so I'd like to fix it soon.

Um sorry, it might be too late. The calendar says Beta->Release migration happens tomorrow.

Luckily there will be 2 additional betas this week, so I'd like to nominate this for 34. [Tracking Requested - why for this release]: The current version checking on www.mozilla.org is inaccurate, confusing visitors with a wrong message, "You’re using the latest version of Firefox." We'd like to offer a better user experience on the site using this new API that provides proper channel/version info. No need to wait for 35. The risk of the patch should be low. Note that a follow-up patch in Bug 1070138 has to be landed at the same time.

This bug doesn't warrant tracking. If you want to uplift, you'll need Beta approval requests on both bugs. The changes in these bugs are really small but I'll note that the extra betas were added to the 34 cycle to stabilize and not to take additional feature work. Although we want this change earlier, I do think that we can wait for this until Firefox 35.

Is there a list somewhere of all possible values for defaultUpdateChannel? If not, could one be provided here to add to the docs (http://bedrock.readthedocs.org/en/latest/uitour.html#getconfiguration-type-callback)?

(In reply to Jon Petto [:jpetto] from comment #22) > Is there a list somewhere of all possible values for defaultUpdateChannel? > If not, could one be provided here to add to the docs > (http://bedrock.readthedocs.org/en/latest/uitour.html#getconfiguration-type- > callback)? There are hundreds of values but the important ones in decreasing order of population are: release, beta, aurora, nightly, default The above should be straightforward except for "default" which refers to self-built or automated testing builds. The are hundreds of variations starting with "release-" for partner builds e.g. the Yahoo! browser bundle. For bug 1027318 I think an exact match for "release" is what you want to test against.