1595641 - [Nudges] Let browser.urlbar.search take an option to control focus in the urlbar input

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P1)

Description

[Drew Willcoxon :adw]

I talked with Verdi today about the nudges experiment. We show a nudge when the user opens the homepage of their default search engine. Verdi says we should not steal focus away from the search box in the homepage when we show the nudge. Currently when we call search (to show the nudge), the urlbar is focused. I guess the best way to implement this is to let browser.urlbar.search take an option to control the focus.

Comment 1

[Drew Willcoxon :adw]

Either that, or we stop focusing on search and add a browser.urlbar.focus function.

Comment 2

[Drew Willcoxon :adw]

Attachment: Bug 1595641 - Add a focus option to browser.urlbar.search and add a browser.urlbar.focus function.

Comment 3

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #1)

Either that, or we stop focusing on search and add a browser.urlbar.focus function.

All the mozilla-central UrlbarInput.search consumers want the urlbar to be focused when they call it. So if we did this, we would probably still want to have a focus option to UrlbarInput.search. The only consumer that would set it to false would be the webext caller.

Comment 4

[Drew Willcoxon :adw]

Actually, we do need a focus function... because when the extension calls search("", { focus: false }) and then the user clicks the "Got it" button in the tip, at that point we should focus the urlbar. It doesn't happen automatically. So the extension will need to add an onResultPicked listener and then call a focus function.

Comment 5

[Marco Bonardo [:mak]]

nvm my previous comment contents... a focus() method is not dangerous.

it's mostly the introduction of an unfocused & expanded state that is a bit scary.

Comment 6

[Shane Caraveo (:mixedpuppy)]

I'm not terribly concerned since this is a privileged experiment api, however it is weird that this focus state cannot be handled internally and would have to rely on the extension to figure out the focus issue. When the nudge is shown, why cant the focus change be avoided?

Comment 7

[Marco Bonardo [:mak]]

We can't handle it internally because it depends on the page that it's loaded. That's an experiment design requirement, on certain pages it wants it focused, on certain other pages not.

It is indeed introducing a state and use for which the urlbar was not designed for.

Comment 8

[Drew Willcoxon :adw]

Just to expand on what Marco said, we actually can avoid the focus change when the tip is shown. That's the purpose of the focus boolean option to search. When we show a tip on google.com, we want the focus to remain in the search box in the page, so the extension calls search("", { focus: false }). We need the focus function because when the user then clicks the button in the tip, the extension needs to focus the input at that time.

(In reply to Marco Bonardo [:mak] from comment #5)

it's mostly the introduction of an unfocused & expanded state that is a bit scary.

I agree it's something to watch out for since it's new behavior. We should keep an eye on whether it causes any unforeseen problems.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/909f5eb0cc48
Add a focus option to browser.urlbar.search and add a browser.urlbar.focus function. r=mak,mixedpuppy

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/909f5eb0cc48