Closed Bug 1562903 Opened 7 months ago Closed 7 months ago

Search engines cannot be added

Categories

(Firefox for Android :: General, defect, P1, major)

Firefox 69
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 70
Tracking Status
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- verified
firefox70 --- verified

People

(Reporter: laurentiu.apahidean, Assigned: sapoliakaran)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Enviroment: Nightly 69.0a1 (2019-07-01)
Devices: Google Pixel C (Android 8.0.0), Huawei Media Pad M3 Lite 10 (Android 7.0), Samsung Galaxy S9 (Android 8.0.0).

Steps to reproduce:

  1. Go to a page that has a non-default search engine (www.imdb.com, www.ebay.com) or a page that has a default search engine that has been deleted from search settings menu (www.google.com, www.bing.com)
  2. Long tap in the search field.
  3. Tap the "Add search engine" option and the "OK" button.

Expected result:
The search engine is added, can be used from the quick search bar and is present in the search settings menu

Actual result:
The search engine is not added

Notes:
The issue does not occur on 68.0 RC.

Using hg bisect with artifact builds I found the regression to be introduced by bug 1553237.
Karan, can you take a look?

Severity: normal → major
Flags: needinfo?(sapoliakaran)
Keywords: regression
Priority: -- → P1
Regressed by: 1553237

Oh, I see what we did here, https://hg.mozilla.org/mozilla-central/rev/45ab583f7acb#l1.12

-        await Services.search.addEngineWithDetails(name, data, null, null, method, formURL);
+        await Services.search.addEngineWithDetails(name, details);

That should have mapped the old parameters into the object, similar to what we did in https://hg.mozilla.org/mozilla-central/rev/365c849f1085#l1.12

Karan, would you have time to fix this, should be fairly simple?

Sure. I'll do that right away.

Flags: needinfo?(sapoliakaran)
Assignee: nobody → sapoliakaran
Status: NEW → ASSIGNED
Attachment #9076876 - Attachment description: Bug 1562903 - Correct regression in 1553237, search engines cannot be added. r?standard8 → Bug 1562903 - Fix adding a search engine from a web page on android. r?standard8
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba18876cfbe9
Fix adding a search engine from a web page on android. r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

Karan, if this bug is a regression in Fennec 69 from bug 1553237, we should uplift your fix to 69 Beta.

Flags: needinfo?(sapoliakaran)

I thought Fennec was being switched to the 68 branch, but uplifting is reasonable to do anyway, just in case.

Flags: needinfo?(sapoliakaran)

Comment on attachment 9076876 [details]
Bug 1562903 - Fix adding a search engine from a web page on android. r?standard8

Beta/Release Uplift Approval Request

  • User impact if declined: Search Engines can't be installed from a web page on Fennec.
  • 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: See comment 0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small change to correct usage of an API after the API was changed.
  • String changes made/needed: None
Attachment #9076876 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9076876 [details]
Bug 1562903 - Fix adding a search engine from a web page on android. r?standard8

Agreed that it's worth taking on Beta as a hedge against any unforeseen changes in strategy, unlikely as that may be. Approved for Beta69.

Attachment #9076876 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Hi!
Verified as fixed on Beta 69.0b4, Nightly 70.0a1 (2019-07-11) with Samsung Galaxy Tab S3 (Android 8), Huawei Honor 8 (Android 7.0).
I will mark this issue as verified on Firefox 69 and Firefox 70.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.