1532761 - visibleDefaultEngines ignored error on beta with browser.search.log

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Mike Kaply [:mkaply]]

Because we're translating the new Google engines after setting up the jarNames list, the engines aren't found in jarNames.

So absearch is getting ignored on beta right now.

Simple fix is to read regionOverrides so that the new engines are in the list.

I'll investigate why this wasn't found by a test as well.

[Tracking Requested - why for this release]: This is needed or we won't read absearch properly.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 1532761 - Add regionOverrides engine to jarNames.

Comment 2

[Mike Kaply [:mkaply]]

This will need a different patch for beta due to refactoring of search.

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Bug 1532761 - Add regionOverrides engine to jarNames.

Comment 4

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

Is anything happening here for 66? Or can this be aimed at 67?

Comment 5

[Mike Kaply [:mkaply]]

Trying to find a reviewer. We really need this for 66. There's a possibility of something requiring absearch. I'm shaking all the trees.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7d8b2c0ada2
Add regionOverrides engine to jarNames. r=daleharvey,adw

Comment 7

[Mike Kaply [:mkaply]]

Comment on attachment 9048659 [details]
Bug 1532761 - Add regionOverrides engine to jarNames.

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1510296
• User impact if declined: Changes from absearch won't be used by Firefox.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: _parseListJSON: ignoring visibleDefaultEngines value because google-b-1-d is not in the jar engines we have found

does not appear in the console.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This was already broken, and this adds a very simple additional check.

I know you are going to ask about automated tests for this, and I'm looking into this, but we need to get this in to make sure absearch is working. Sorry for the lateness, found only in testing and had some review issues.

• String changes made/needed:

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/c7d8b2c0ada2

Comment 9

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

From discussion with Mike this morning, this is a release blocker.

Comment 10

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

If this was regressed by bug 1510296, does this issue also affect esr60?

Comment 11

[Mike Kaply [:mkaply]]

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #10)

If this was regressed by bug 1510296, does this issue also affect esr60?

Probably, but I was focused on release. I'm not as concerned their.

I'm still investigating how we could have caught this with a test. The problem is that this is specifically an interaction with the absearch server which we can't check via automated tests (and the error message only shows when we are in debug)

Comment 12

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

Does this also affect Fennec? We should likely also test there.

Comment 13

[Mike Kaply [:mkaply]]

Does this also affect Fennec? We should likely also test there.

The search code is the same between desktop and Fennec, so it would be fixed there as well.

The browser.search.log thing should work for Fennec as well, but you'd have to look at an adb log to see it.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

From the email thread, it sounds like we can punt on this until 67. Tracking for ESR60 accordingly.

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-release/rev/2c906ef692e7

Comment 16

[Mike Kaply [:mkaply]]

Reproduction scenario:

Go to about:config, flip browser.search.log to true

Restart the browser

Bring up the JS console. Search for visibleDefaultEngines

You'll see this error:

_parseListJSON: ignoring visibleDefaultEngines value because google-b-1-d is not in the jar engines we have found

After the fix, that error will not be there.

Comment 17

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx 67.0a1 (2019-03-05), on Windows 10.
I can confirm this issue is fixed, I verified using Fx 67.0a1 (2019-03-14)and Fx 66.0-build2, on Windows 10 x64, macOS 10.13 and Ubuntu 16.04 LTS

Comment 18

[Andrei Bodea[:andrei]]

Hello, I can confirm that this issue cannot be reproduced on Fennec on the latest Nightly 67.0a1 with Samsung Galaxy J7(Android 6.0).

Comment 19

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

Can you also request uplift to ESR please?

Comment 20

[Mike Kaply [:mkaply]]

Attachment: ESR patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes ability to get server search engines
• User impact if declined: Changes from absearch won't be used by Firefox.
• Fix Landed on Version: 67
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low touch, easy to test.
• String or UUID changes made by this patch: None

Comment 21

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

Comment on attachment 9057398 [details] [diff] [review]
ESR patch

Search code fix, verified in nightly, ok for ESR uplift.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/b5e6b61e7bfd

Comment 23

[Firefox Bug Husbandry Bot]

Please specify a root cause for this bug. See :tmaity for more information.