Ship Baidu searchplugin code update via SAO to Firefox 45+
Categories
(Firefox :: Search, enhancement)
Tracking
()
People
(Reporter: hectorz, Unassigned)
References
Details
(Whiteboard: [go-faster-system-addon])
Attachments
(5 files)
As seen in bug 1540017, Baidu is asking us to replace the search code "monline_dg" in vanilla desktop Firefox with a new one "monline_7_dg".
In addition to the in-tree update, we'd like to use system addons to make the same change in previous versions of Firefox.
Reporter | ||
Comment 1•6 years ago
|
||
Extensions based on google-code-correction was sent in this PR for review.
Reporter | ||
Comment 2•6 years ago
|
||
Reporter | ||
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
Comment on attachment 9055387 [details]
Unsigned baidu-code-update-2.66.xpi, for Fx 60+, webext with experiment
Please sign this system add-on update, thanks!
Comment 5•6 years ago
|
||
Comment on attachment 9055388 [details]
Unsigned baidu-code-update-1.66.xpi, for Fx 45-60, bootstrapped
Please sign this system add-on update, thanks!
Comment 8•6 years ago
|
||
Rehan, could you please host both XPIs attache to this bug on the test channel? Thanks!
Comment 9•6 years ago
|
||
The 1.66 XPI is now on release-sysaddon 45.* through 59.*
The 2.66 XPI is now on release-sysaddon 60.* through 66.*
Updated•6 years ago
|
Updated•6 years ago
|
Comment 10•6 years ago
|
||
Hector, this is ready to ship and you mentioned to me that this already went through QA but it would be worth double-checking by:
- changing the Firefox update channel to
release-sysaddon
- in
about:config
, setextensions.logging.enabled
totrue
- in the Browser Console, run:
Cu.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck();
I'll run through this as well and ensure that we're serving up the right XPIs, but I thought you might better be able to tell if it is having the intended effect.
Rehan, per the "intent to ship" email that Hector sent this should be targeted to the zh-CN locale only, would you mind making this change?
Comment 11•6 years ago
|
||
It is possible --- but extremely difficult (and potentially buggy) --- to filter by locale in Balrog.
If we can avoid that it would be my strong preference.
At present we are recommending that addons be shipped to 100% of users and use Normandy to filter with a pref flip. This pattern has proved to be more flexible in terms of how we filter (including geo-targeting) and less error-prone.
@Hector would we be able to go down this route instead of filtering by locale in Balrog?
Reporter | ||
Comment 12•6 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #10)
Hector, this is ready to ship and you mentioned to me that this already went through QA but it would be worth double-checking by:
- changing the Firefox update channel to
release-sysaddon
- in
about:config
, setextensions.logging.enabled
totrue
- in the Browser Console, run:
Cu.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck();
I'll run through this as well and ensure that we're serving up the right XPIs, but I thought you might better be able to tell if it is having the intended effect.
Thanks for the instructions. We might not be able to cover all versions during the weekend, but we’ll try this on Fx 66/60esr/52esr at the very least.
Reporter | ||
Comment 13•6 years ago
|
||
(In reply to Rehan Dalal [:rehan, :rdalal] from comment #11)
It is possible --- but extremely difficult (and potentially buggy) --- to filter by locale in Balrog.
If we can avoid that it would be my strong preference.
At present we are recommending that addons be shipped to 100% of users and use Normandy to filter with a pref flip. This pattern has proved to be more flexible in terms of how we filter (including geo-targeting) and less error-prone.
@Hector would we be able to go down this route instead of filtering by locale in Balrog?
Current versions will return early for non-zh-CN locale builds since they don’t ship with a Baidu searchplugin. I’m actually okay with shipping them to 100% since system addons are not visible to ordinary users in about:addons . If you have concerns about the unnecessary additional observer, I can update the patch to include a locale check first thing Monday morning.
Reporter | ||
Comment 14•6 years ago
|
||
(In reply to Hector Zhao [:hectorz] from comment #12)
Thanks for the instructions. We might not be able to cover all versions during the weekend, but we’ll try this on Fx 66/60esr/52esr at the very least.
And I’ve verified by following the instructions, Baidu searchplugins are updated as expected in the three Fx versions mentioned above.
Comment 15•6 years ago
|
||
If we're pushing this to non-zh-CN locales, should we have our regular QA team take a look?
Comment 16•6 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #15)
If we're pushing this to non-zh-CN locales, should we have our regular QA team take a look?
That would be ideal to look out for any unintended side effects.
(In reply to Hector Zhao [:hectorz] from comment #13)
Current versions will return early for non-zh-CN locale builds since they don’t ship with a Baidu searchplugin. I’m actually okay with shipping them to 100% since system addons are not visible to ordinary users in about:addons . If you have concerns about the unnecessary additional observer, I can update the patch to include a locale check first thing Monday morning.
I am not too concerned with the additional observer, especially if QA is taking a look.
Comment 17•6 years ago
|
||
Mike, Reese, shipping this system add-on to all Firefox users means it will show up in about:support.
I want to check in with you about that, since many users will notice the change.
We should likely talk with the comms team to plan to explain this to users.
Updated•6 years ago
|
Comment 18•6 years ago
|
||
Hello,
I checked the following versions:
- 45 ru (Windows 10 x64)
- 53 fr (Windows 10 x64)
- 59 pl (mac OS 10.13.6)
- 60 ru (Windows 10 x64)
- 63 es-ES (Windows 10 x64)
- 66 ru (mac OS 10.13.6)
The search codes were all correct and the builds older than 60 showed 1.66 XPI and for builds newer than 60 showed 2.66 Xpi.
Reporter | ||
Comment 19•6 years ago
|
||
(In reply to Rehan Dalal [:rehan, :rdalal] from comment #9)
The 1.66 XPI is now on release-sysaddon 45.* through 59.*
The 2.66 XPI is now on release-sysaddon 60.* through 66.*
I think this could be further limited to Fx 60.0 - 66.0.2, now that the in-tree patch was uplifted into Fx 66.0.3 (bug 1540017 comment 17)
Comment 20•5 years ago
|
||
Is this now ready to ship since we have QA signoff?
Comment 21•5 years ago
|
||
From Hector in email:
With the about:support concerns gone, I'd say the extensions are ready to be shipped.
The code is based on
google-code-correction
around Fx 57 and reviewed by :rhelmer;
The extensions were verified against each and every compatible Fx versions by Yanfang Liu, desktop QA in Beijing office and they work as expected;
The installation through release-sysaddon channel was verified by release QA :dcicas, who also verified the extensions have not inadvertently changed any other searchplugins.
Mike, who gives the final say here for implementation and what is your planned timing?
We only have a couple more weeks left in the 66 release cycle. I'd like to avoid this launch falling in the most active part of the 67 cycle (i.e. RC week).
Comment 22•5 years ago
|
||
Bridget, can you help clarify the plan for QA and for launch? Thanks!
Comment 24•5 years ago
|
||
Rehan, I think this is ready to ship at this point. Is there any other signoff that you need?
Comment 25•5 years ago
|
||
Does this need to be confidential? Bug 1540017 is not.
Updated•5 years ago
|
Comment 26•5 years ago
|
||
@mconnor, please confirm that the QA performed is sufficient in testing potential neg impact of our regl. browser activity (non-CN)
Reporter | ||
Comment 27•5 years ago
|
||
(In reply to Liz Henry (:lizzard) (use needinfo) from comment #25)
Does this need to be confidential? Bug 1540017 is not.
Not really, opening this up.
Comment 28•5 years ago
|
||
Based on what Hector's said in email, and what I've seen of the code, I'm confident the QA performed is sufficient (and risk is appropriately scoped to the Baidu engine only).
Comment 29•5 years ago
|
||
Shipped in bug 1548578.
Comment 30•5 years ago
|
||
From conversation with Hector, we are going to want to ship this to ESR users as well.
Note that uptake for ESR users is often low (as these installs are often in enterprise environments where system addons are not accepted). ESR usage for the population we want to get baidu's update to may vary, of course!
Comment 31•5 years ago
|
||
signed.baidu-code-update-1.66.xpi no longer installs on ESR 60.0.2 or earlier because of armagadd-on-2.0.
Comment 32•5 years ago
|
||
signed.baidu-code-update-2.66.xpi did not install, either.
Comment 33•5 years ago
|
||
OK. There is a bug/project underway to ship a hotfix to older versions for the cert2019 issues; let me find that bug and connect this as a dependency as, once that fix is out, we can ship this one.
Comment 34•5 years ago
|
||
That will only get to users from 52 and up. So, users on ESR versions before 52 may not be able to get the baidu update.
Comment 35•5 years ago
|
||
We will have to re-sign this SAO for 52-60. We can't depend on bug 1549604 because this SAO prevents from installing other SAOs (including a hotfix from bug 1549604).
Comment 36•5 years ago
|
||
Bridget can you help out here to move this along or find someone to be responsible for it?
I notice that it is still unassigned. Thanks!
Comment 37•5 years ago
|
||
Hector, are you working on this or should it be assigned to someone else?
Mike, Bridget asked me to pass this along to you as she is busy this week with onboarding.
Updated•5 years ago
|
Reporter | ||
Comment 38•5 years ago
|
||
Sorry I only tested v2.66 on Fx 60esr and missed bug 1454820 which means it might not work on Fx 60 release.
Estimation of current status, of all release & esr channel zh-CN users:
- ~78% users already updated to Fx 66.0.3+, with the fix in bug 1540017;
- ~5% users on Fx 61+, should be covered by v2.66 shipped in bug 1548578;
- <1% users on Fx 60, I think we should switch them to v1.66 due to the above-mentioned bug;
- ~10.5% users on Fx 52esr or Fx 52.0 ~ 59.*, I'd like to ship v1.66, but this needs a new signature, see bug 1541316 comment 35 above;
- ~2.5% users on Fx 45esr or Fx 45.0 ~ 51.*, if we can also ship v1.66 to them;
- ~2% users on Fx 60esr, should be fixed by tomorrow's 60.7.0 update.
Actions we could take:
- Change v2.66 to ship in Fx 61.0 ~ 66.0.2, instead of the current Fx 60.0 ~ 66.*;
- Re-sign and ship v1.66 to Fx 52.0 ~ 60.* on both release and esr channels;
- Extend v1.66 to Fx 45.0 ~ 60.* if possible.
Reporter | ||
Comment 39•5 years ago
|
||
(In reply to Hector Zhao [:hectorz] from comment #38)
Actions we could take:
- Change v2.66 to ship in Fx 61.0 ~ 66.0.2, instead of the current Fx 60.0 ~ 66.*;
- Re-sign and ship v1.66 to Fx 52.0 ~ 60.* on both release and esr channels;
- Extend v1.66 to Fx 45.0 ~ 60.* if possible.
It should be Fx 60.6.* to avoid installing this system addon for those already updated to Fx 60.7.0+
Comment 40•5 years ago
|
||
Why are you trying to ship to 45? is there really that much volume there?
rhelmer is the right person for this discussion since he had to go through this pain for the addon fix.
Comment 41•5 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #40)
Why are you trying to ship to 45? is there really that much volume there?
rhelmer is the right person for this discussion since he had to go through this pain for the addon fix.
45 is technically how far back we can go with system add-ons, but yes I'd like to see data justifying going back this far. The QA burden is quite high, and there is a cost to shipping add-ons that don't install (it keeps others in the set from installing, by design)
Reporter | ||
Comment 42•5 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #41)
(In reply to Mike Kaply [:mkaply] from comment #40)
Why are you trying to ship to 45? is there really that much volume there?
Fx 45 was originally chosen mostly because it's the first ESR compatible with google-code-correction.
......
45 is technically how far back we can go with system add-ons, but yes I'd like to see data justifying going back this far. The QA burden is quite high, and there is a cost to shipping add-ons that don't install (it keeps others in the set from installing, by design)
v1.66 was verified against Fx 45-60 by our desktop QA, Yanfang Liu. It should install fine with no code change but an updated signature.
(In reply to Hector Zhao [:hectorz] from comment #38)
Estimation of current status, of all release & esr channel zh-CN users:
"Estimation" because these ratios were calculated based on profiles with Fx China repack. I don't know if the search related telemetry dataset goes back that far, but I think I can get Fx versions based on profiles with vanilla zh-CN builds.
......
- ~2.5% users on Fx 45esr or Fx 45.0 ~ 51.*, if we can also ship v1.66 to them;
......- Extend v1.66 to Fx 45.0 ~ 60.* if possible.
I understand shipping to Fx 45.0 ~ 51.* may not worth the effort, so I listed it separately below the Fx 52.0 ~ 60.6.* one.
Reporter | ||
Comment 43•5 years ago
|
||
(In reply to Hector Zhao [:hectorz] from comment #42)
...... but I think I can get Fx versions based on profiles with vanilla zh-CN builds.
Please have a look at https://sql.telemetry.mozilla.org/queries/62888#161365 , if I did the SQL correctly, about 3.8% of vanilla zh-CN users are using Fx 45.0 ~ 51.*.
Comment 44•5 years ago
|
||
Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1553936. This is what we are installing to complete this Baidu install. Discussed between mconnor, RituK and Rdalal.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 45•5 years ago
|
||
AFAICT, this missed 60.8esr at this point. Should be trying to get this into 60.9esr this cycle still?
Comment 46•5 years ago
|
||
Mike, can you help shepherd this and related bugs?
Comment 47•5 years ago
|
||
Now too late for 60.9.
Updated•5 years ago
|
Description
•