1661949 - No longer able to search from address bar with ? prefix and keyword.enabled = false

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[nuser97]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

Set keywords.enabled = false
Type search text in the address bar, prefixed with a question mark

Actual results:

In Firefox 79, text is sent to the default search engine
In Firefox 80, receive the "Hmm. That address doesn’t look right." error

Expected results:

I expect the keywords.enabled setting only to apply for implicit searches, where no question mark prefix is specified, as was the case in Firefox 79 and earlier. When explicitly searching with the question mark prefix, the text should be sent to the default search engine.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Alice0775 White]

I can reproduce the issue if set keyword.enabled=false.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=52aae6784eb346ee86af8d1f13cdb8965c3a47be&tochange=51a157c5fe34d465fcd08a8f4d5276ae13c06f1c

Comment 4

[Marco Bonardo [:mak]]

I can look at this, it is probably a regression due to the big refactoring in Bug 1648468 or Bug 1645521.

Comment 5

[Marco Bonardo [:mak]]

Attachment: Bug 1661949 - Can't search from the Address Bar with ? if keyword.enabled = false. r=adw

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0eacf15bc150
Can't search from the Address Bar with ? if keyword.enabled = false. r=adw

Comment 7

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 0eacf15bc150 (bug 1661949) for xpcshell failures at test_416211.js.
https://hg.mozilla.org/integration/autoland/rev/645d922a048ce0d9ef0a3ba6d50ce9f05011f80a

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=0eacf15bc150050afeaaeee067f042ce6ac576ff&selectedTaskRun=LMLDtnpMTJejcla7PlP3Pg.0

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314777954&repo=autoland&lineNumber=4611

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef2483cfdf64
Can't search from the Address Bar with ? if keyword.enabled = false. r=adw

Comment 9

[Narcis Beleuzu [:NarcisB]]

Backed out for xpcshell failures on test_search_suggestions.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/f4b764b069ffcf94c1759c18bf2365bc3d796633
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314860987&repo=autoland&lineNumber=3888

Comment 10

[Marco Bonardo [:mak]]

the landing of bug 1658964 may have broken my patch, I could not reproduce that failure before.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/711b8615bb3f
Can't search from the Address Bar with ? if keyword.enabled = false. r=adw

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/711b8615bb3f

Comment 13

[Marco Bonardo [:mak]]

Beta will require a different patch because the code differs.

Comment 14

[Marco Bonardo [:mak]]

Attachment: Beta patch

Approval Request Comment
[Feature/Bug causing the regression]: Urlbar code refactoring in bug 1645521
[User impact if declined]: Setting keyword.enabled to false doesn't allow to force searching in the urlbar with ?
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: yes, see comment 0
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: medium
[Why is the change risky/not risky?]: The affected behaviors are minimal, but of course there's always risk of unknown side effects, we have a lot of tests for the urlbar though
[String changes made/needed]:

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9174839 [details] [diff] [review]
Beta patch

Can't say I'm crazy about taking this so late in the cycle, but the impact seems not-great for affected users. Thanks for adding a new test for this. Approved for 81.0b9.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/780fe058bccd

Comment 17

[Alexandra Martin]

Confirmed as verified fixed on Windows 10 x64bit and MacOS 10.14 with latest Nightly 82.0a1 (20200910152110) and Beta 81.0b9 (20200910180444).