Closed Bug 1595641 Opened 4 months ago Closed 4 months ago

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

Categories

(Firefox :: Address Bar, task, P1)

task
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 72
Iteration:
72.2 - Nov 4 - 17
Tracking Status
firefox72 --- fixed

People

(Reporter: adw, Assigned: adw)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.

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

(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.

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

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.

See Also: → 1595653

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.

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?

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.

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.

Pushed by dwillcoxon@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72
You need to log in before you can comment on or make changes to this bug.