Closed Bug 1293617 Opened 8 years ago Closed 8 years ago

[Out Of Date Notification] Notification bar is displayed for partner repack builds

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: aflorinescu, Assigned: bhearsum)

References

(Blocks 1 open bug)

Details

[Installation Notes]:
At the date on which this bug was logged, the access to actual partner builds is not available, but we were provided a dropbox link to download and test on a few:

acer-Firefox Setup 44.0.1.exe
aol-en-US-Firefox 44.0.dmg
aol-en-US-Firefox Setup 44.0.exe 
webde-de-Firefox 44.0.2.dmg 
webde-de-Firefox Setup 44.0.2.exe 
yahoo-en-US-Firefox 44.0.dmg 
yahoo-en-US-Firefox Setup 44.0.exe 


[Description]:
Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1280378#c51 , the partner repacks shouldn't be affected by the OutOfDate notification system addon. Our understanding was that at least for this release, the partner builds would be ignored.


[Affected OS]:
We tested on Windows 8.1 and OSX 10.11 and normally it would affect all Operating systems. (not sure if there are partner repacks for Linux distropies)

[Steps]:
1. Install a Firefox partner repacked version 44.*.

2. Set Outofdate notifications system add-on required configurations:
2.1 about:config set extensions.systemAddon.update.url to value 	"https://aus5.mozilla.org/update/3/SystemAddons/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/release-sysaddon/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml"
2.2 about:config set extensions.logging.enabled	to value "True"
2.3 about:config Add string app.update.url.override with value "www.softvision.ro" 
3. Set update preferences ->  about:preferences#advanced and set Firefox updates to "Automatically install updates (recommended: improved security)"
4. Open Tools/Web console and click on the gear icon to get to settings, then enable the following two options: "Enable browser chrome and add-on debugging toolboxes" and "Enable remote debugging".
5. Force system addons check : Open Tool/WebDeveloper/Browser Console and run the snippet:   " Components.utils.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck(); "
6. Restart FF.
7. Click on the Download Firefox OutofDate Notification and check the download link.

[Actual Result]:
Step 6: The OutOfDate system addon is installed and ran. The OutOfDate notification bar is displayed.
Step 7: The OutOfDate system addon download button will redirect you to download a stock Firefox versioned at it's last version.

[Expected Result]:
The Partner Repack builds should ignore the OutOfDate Notification bar or the Download Firefox should redirect you in this case to a link that will allow you to download an up to date partner repack.

[Note]:
Not sure how many partner repacks there are out there for the 44.* builds or what exactly is the telemetry data with regards to partner repacks and version 44.*.
I was able to confirm this with yahoo-en-US-Firefox 44.0.dmg. However, using the browser console and executing the same code as in the addon, we properly detect the build as a partner build and should not be showing the outofdate notification. Both Felipe and I are currently out of ideas what could be causing this.

Since we're sending %DISTRIBUTION% as part of the system addon update check URL, we're wondering if we could just not ship the addon to partner builds. Nick, would that be possible? The list of partner builds to avoid shipping the addon to would be partner builds that have a %DISTRIBUTION% value that start with one of the following:

"1und1",
"acer",
"aol",
"bing",
"gmx",
"mail.com",
"toshiba",
"webde",
"yandex",
"yahoo"

Is this something that we can do via Balrog?
Flags: needinfo?(nthomas)
(In reply to Stephen A Pohl [:spohl] from comment #1)
> I was able to confirm this with yahoo-en-US-Firefox 44.0.dmg. However, using
> the browser console and executing the same code as in the addon, we properly
> detect the build as a partner build and should not be showing the outofdate
> notification. Both Felipe and I are currently out of ideas what could be
> causing this.
> 
> Since we're sending %DISTRIBUTION% as part of the system addon update check
> URL, we're wondering if we could just not ship the addon to partner builds.
> Nick, would that be possible? The list of partner builds to avoid shipping
> the addon to would be partner builds that have a %DISTRIBUTION% value that
> start with one of the following:
> 
> "1und1",
> "acer",
> "aol",
> "bing",
> "gmx",
> "mail.com",
> "toshiba",
> "webde",
> "yandex",
> "yahoo"
> 
> Is this something that we can do via Balrog?

Yeah, shouldn't be a problem. I'd recommmend inverting this and matching distribution = default" though, to make it a bit less painful.
Spoke with mkaply on IRC. We can go ahead and just ship to "default" for now. We always have the ability to be smarter about this in a future release.
Assignee: nobody → bhearsum
Flags: needinfo?(nthomas)
I tweaked the Balrog rules to only ship this to 44.0 w/ "default" as the distribution. Should be fixed now.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Sounds like a solid solution to me. Did you consider a startup ordering issue for not detecting the partner prefs ?
(In reply to Nick Thomas [:nthomas] from comment #5)
> Sounds like a solid solution to me. Did you consider a startup ordering
> issue for not detecting the partner prefs ?

No, I haven't. Robert, is this possibly the problem here? Do we not have access to distribution.id during startup of system addons?
Flags: needinfo?(rhelmer)
(In reply to Stephen A Pohl [:spohl] from comment #6)
> (In reply to Nick Thomas [:nthomas] from comment #5)
> > Sounds like a solid solution to me. Did you consider a startup ordering
> > issue for not detecting the partner prefs ?
> 
> No, I haven't. Robert, is this possibly the problem here? Do we not have
> access to distribution.id during startup of system addons?

Hm, I don't know enough about how this works to say - I just take a look and I think it's done in here:
https://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js

Stephen, does it work if you add an observer for "distribution-customization-complete" before trying to get the "distribution.id" pref?
Flags: needinfo?(rhelmer) → needinfo?(spohl.mozilla.bugs)
Once the preferences are set, they are set for good, so distribution customization shouldn't matter.

We should be testing this with a distribution that was installed, run, then on subsequent loads got the update.

There won't be a case where someone gets the add-on on the first run of the partner build...
(In reply to Robert Helmer [:rhelmer] from comment #7)
> (In reply to Stephen A Pohl [:spohl] from comment #6)
> > (In reply to Nick Thomas [:nthomas] from comment #5)
> > > Sounds like a solid solution to me. Did you consider a startup ordering
> > > issue for not detecting the partner prefs ?
> > 
> > No, I haven't. Robert, is this possibly the problem here? Do we not have
> > access to distribution.id during startup of system addons?
> 
> Hm, I don't know enough about how this works to say - I just take a look and
> I think it's done in here:
> https://dxr.mozilla.org/mozilla-central/source/browser/components/
> nsBrowserGlue.js
> 
> Stephen, does it work if you add an observer for
> "distribution-customization-complete" before trying to get the
> "distribution.id" pref?

This requires resigning and restaging the addon, correct? Or is there an easier way to test this?
Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(rhelmer)
(In reply to Mike Kaply [:mkaply] from comment #8)
> Once the preferences are set, they are set for good, so distribution
> customization shouldn't matter.
> 
> We should be testing this with a distribution that was installed, run, then
> on subsequent loads got the update.
> 
> There won't be a case where someone gets the add-on on the first run of the
> partner build...

We currently have to restart Firefox after a system addon is installed before system addons start up for the first time. And we don't seem to detect that we're started by a partner repack when this happens.

Also, I can confirm that subsequent restarts of Firefox still display the doorhanger.
Michael, does this run on every startup?
http://searchfox.org/mozilla-central/rev/d83f1528dbcb1e837d99cf5b6c36a90b03bf0e77/browser/components/distribution.js#331

I'm guessing yes, and then that would explain why the add-on didn't work as expected (because it starts up earlier than this code).
Flags: needinfo?(mozilla)
(In reply to Stephen A Pohl [:spohl] from comment #9)
> (In reply to Robert Helmer [:rhelmer] from comment #7)
> > (In reply to Stephen A Pohl [:spohl] from comment #6)
> > > (In reply to Nick Thomas [:nthomas] from comment #5)
> > > > Sounds like a solid solution to me. Did you consider a startup ordering
> > > > issue for not detecting the partner prefs ?
> > > 
> > > No, I haven't. Robert, is this possibly the problem here? Do we not have
> > > access to distribution.id during startup of system addons?
> > 
> > Hm, I don't know enough about how this works to say - I just take a look and
> > I think it's done in here:
> > https://dxr.mozilla.org/mozilla-central/source/browser/components/
> > nsBrowserGlue.js
> > 
> > Stephen, does it work if you add an observer for
> > "distribution-customization-complete" before trying to get the
> > "distribution.id" pref?
> 
> This requires resigning and restaging the addon, correct? Or is there an
> easier way to test this?

Yes it does unfortunately. If you're doing your own build it's easy to disable the check in the source (I guess you could do it by hacking omni.ja in a release build too, if that's actually easier :) )
I think the balrog solution is good enough for this (as long as it tests).

distribution-customization-complete shouldn't matter here because the pref has already been set in subsequent starts.
Flags: needinfo?(mozilla)
Flags: needinfo?(rhelmer)
Verified the Balrog fix on the partner builds (comment0) on win 8.1 and Mac OSX 10.11 and we confirm that the OutOfDate notification system add-on is not installed and activated on the available partner builds.

Also, we can also confirm that the OutOfDate system add-on is removed if the partner build is opened with a profile that had the OutOfDate notification system add-on installed.
Status: RESOLVED → VERIFIED
Blocks: 1294249
(In reply to Mike Kaply [:mkaply] from comment #13)
> distribution-customization-complete shouldn't matter here because the pref
> has already been set in subsequent starts.

distribution.id etc. are not persistent, they're set on default branch after each start.
> distribution.id etc. are not persistent, they're set on default branch after each start.

Doh. Good point. If this ever gets revived, need to remember that.
You need to log in before you can comment on or make changes to this bug.