1518514 - Search Engine reporting not working on Fennec

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mike Kaply [:mkaply]]

Fennec isn't reporting the default search engine.

It's also showing search related errors on the console (though everything is working)

I think this was caused by:

https://bugzilla.mozilla.org/show_bug.cgi?id=1461432

When we moved away from browsersearch.json, list.json was no longer in the locale JAR file, so we're trying to read it from the wrong location.

We can't read a resource file in SearchEngineManager.java, so we need to create a custom content URL so we can read it.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 1518514 - Correctly read and parse list.json in search engine manager.

Comment 2

[Mike Kaply [:mkaply]]

I was right. In bug 1461432, we did the switch but didn't realize the locale implications.

Because the browser itself uses the search service in JS, this didn't have any side effects except to break reporting. (Everything worked correct in the browser).

This patch switches list.json to be accessible via a chrome JAR URL and properly parses it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1522f31ad7d
Correctly read and parse list.json in search engine manager. r=nalexander

Comment 4

[Cristian Brindusan [:cbrindusan]]

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

Comment 5

[Mike Kaply [:mkaply]]

Comment on attachment 9035173 [details]
Bug 1518514 - Correctly read and parse list.json in search engine manager.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1461432

User impact if declined: No user impact, reporting only.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: This will be verified in reporting.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The function wasn't working at all, so this just makes it work.

String changes made/needed:

Comment 6

[Ryan VanderMeulen [:RyanVM]]

I'd feel a lot better about taking this if it had some sort of test coverage.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9035173 [details]
Bug 1518514 - Correctly read and parse list.json in search engine manager.

[Triage Comment]
I'm going to approve this for 65.0b12 because it fixes a function which is currently broken. NI mkaply to verify that 65.0b12 is properly reporting this information after it ships on Friday.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/21f066e22f2a

Comment 9

[Mike Kaply [:mkaply]]

I'm going to approve this for 65.0b12 because it fixes a function which is currently broken. NI mkaply to verify that 65.0b12 is properly reporting this information after it ships on Friday.

Sadly it's not. I have one more fix that will have to wait for 66.