1541316 - Ship Baidu searchplugin code update via SAO to Firefox 45+

Status: RESOLVED FIXED

(Firefox :: Search, enhancement)

Description

[Hector Zhao [:hectorz]]

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.

Comment 1

[Hector Zhao [:hectorz]]

Attachment: GitHub Pull Request

Extensions based on google-code-correction was sent in this PR for review.

Comment 2

[Hector Zhao [:hectorz]]

Attachment: Unsigned baidu-code-update-2.66.xpi, for Fx 60+, webext with experiment

Comment 3

[Hector Zhao [:hectorz]]

Attachment: Unsigned baidu-code-update-1.66.xpi, for Fx 45-60, bootstrapped

Comment 4

[Robert Helmer [:rhelmer]]

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

[Robert Helmer [:rhelmer]]

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 6

[:wezhou]

Attachment: signed.baidu-code-update-1.66.xpi

Signed file attached. Please test.

Comment 7

[:wezhou]

Attachment: signed.baidu-code-update-2.66.xpi

The second signed file attached. Please test.

Comment 8

[Robert Helmer [:rhelmer]]

Rehan, could you please host both XPIs attache to this bug on the test channel? Thanks!

Comment 9

[Rehan Dalal (Inactive)]

The 1.66 XPI is now on release-sysaddon 45.* through 59.*
The 2.66 XPI is now on release-sysaddon 60.* through 66.*

Comment 10

[Robert Helmer [:rhelmer]]

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:

1. changing the Firefox update channel to release-sysaddon
2. in about:config, set extensions.logging.enabled to true
3. 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

[Rehan Dalal (Inactive)]

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?

Comment 12

[Hector Zhao [:hectorz]]

(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:

1. changing the Firefox update channel to release-sysaddon
2. in about:config, set extensions.logging.enabled to true
3. 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.

Comment 13

[Hector Zhao [:hectorz]]

(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.

Comment 14

[Hector Zhao [:hectorz]]

(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

[Julien Cristau [:jcristau]]

If we're pushing this to non-zh-CN locales, should we have our regular QA team take a look?

Comment 16

[Rehan Dalal (Inactive)]

(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

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 18

[Daniel Cicas [:dcicas], Release QA]

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.

Comment 19

[Hector Zhao [:hectorz]]

(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

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this now ready to ship since we have QA signoff?

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

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

[Liz Henry (:lizzard) (relman/hg->git project)]

Bridget, can you help clarify the plan for QA and for launch? Thanks!

Comment 23

[Bridget Kaluzny]

Working on this - thanks for the ping Lizzard.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Rehan, I think this is ready to ship at this point. Is there any other signoff that you need?

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

Does this need to be confidential? Bug 1540017 is not.

Comment 26

[Bridget Kaluzny]

@mconnor, please confirm that the QA performed is sufficient in testing potential neg impact of our regl. browser activity (non-CN)

Comment 27

[Hector Zhao [:hectorz]]

(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

[Mike Connor [:mconnor]]

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

[Liz Henry (:lizzard) (relman/hg->git project)]

Shipped in bug 1548578.

Comment 30

[Liz Henry (:lizzard) (relman/hg->git project)]

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

[Masatoshi Kimura [:emk]]

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

[Masatoshi Kimura [:emk]]

signed.baidu-code-update-2.66.xpi did not install, either.

Comment 33

[Liz Henry (:lizzard) (relman/hg->git project)]

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

[Liz Henry (:lizzard) (relman/hg->git project)]

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

[Masatoshi Kimura [:emk]]

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

[Liz Henry (:lizzard) (relman/hg->git project)]

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

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 38

[Hector Zhao [:hectorz]]

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.

Comment 39

[Hector Zhao [:hectorz]]

(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

[Mike Kaply [:mkaply]]

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

[Robert Helmer [:rhelmer]]

(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)

Comment 42

[Hector Zhao [:hectorz]]

(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.

Comment 43

[Hector Zhao [:hectorz]]

(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

[Bridget Kaluzny]

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.

Comment 45

[Ryan VanderMeulen [:RyanVM]]

AFAICT, this missed 60.8esr at this point. Should be trying to get this into 60.9esr this cycle still?

Comment 46

[Ryan VanderMeulen [:RyanVM]]

Mike, can you help shepherd this and related bugs?

Comment 47

[Julien Cristau [:jcristau]]

Now too late for 60.9.