Closed Bug 1507568 Opened 5 years ago Closed 5 years ago

Clicking a @shortcut in the urlbar popup should fill it in the input instead of visiting the site

Categories

(Firefox :: Address Bar, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 65
Tracking Status
firefox64 --- verified
firefox65 --- verified

People

(Reporter: adw, Assigned: adw)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

When you type @, you get a list of all the @shortcuts.  Right now, if you click one of those shortcuts, you visit the site's search page.  Instead, we should fill in the @shortcut in the urlbar input so that you can type a search query and do a search from the urlbar.  (I mention "when you type @", but this should happen any time there are @shortcuts in the popup results, e.g., if you type "@a" and there happen to be two shortcuts that start with "a" that are shown in the popup (bug 1502930).)

I'm filing this based on a meeting Verdi and I had just now about @shortcuts improvements.
Assignee: adw → nobody
Status: ASSIGNED → NEW
Assignee: nobody → adw
Status: NEW → ASSIGNED
Verdi, this patch treats clicks and return key presses the same.  That is, if you use the arrow keys to highlight an @shortcut and fill it in the input, and then if you hit the return key, we don't actually do anything.  It's as if you'd clicked the @shortcut.

Is that what we want?  I know we wanted clicks to behave that way, but we didn't talk about return key presses.  It would be very easy to change the patch so that only clicks behave like that.

My thinking for having the patch work the way it does is that I bet a lot of people would key down to a shortcut and then hit return without typing anything, and then they'd actually do their search in the web page.  Maybe if we just don't respond, they'll take the hint.  Or maybe if you're savvy enough to use @shortcuts, you wouldn't actually do that?
Flags: needinfo?(mverdi)
(In reply to Drew Willcoxon :adw from comment #2)
> Verdi, this patch treats clicks and return key presses the same.  That is,
> if you use the arrow keys to highlight an @shortcut and fill it in the
> input, and then if you hit the return key, we don't actually do anything. 
> It's as if you'd clicked the @shortcut.
> 
> Is that what we want?

Yes. That sounds right. Thanks Drew.
Flags: needinfo?(mverdi)
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efa49008bfc7
Clicking a @shortcut in the urlbar popup should fill it in the input instead of visiting the site. r=mak
Flags: qe-verify+
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/efa49008bfc7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
STR for QA:

1. Type "@" in the urlbar
2. Click any one of the @ results in the popup, @google for example
3. "@google " should be filled in the urlbar textbox
4. The popup will change and maybe flicker, and it should ultimately show a single "Search with Google" result
5. Repeat steps 1-3 except in step 2 instead of clicking, use the keyboard to highlight @google and then press the return key
6. Nothing should happen -- Firefox should not visit Google -- except that the popup will maybe flicker, and it should show a single "Search with Google" result
Attached patch Beta/64 patchSplinter Review
[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Not a regression but improvement to search shortcuts in the awesomebar: see bug 1496772

User impact if declined: We've landed a group of improvements to search shortcuts in 64 (see bug 1496772) and it would be great to have this bug, too. It's an especially good bang for the buck considering the big improvement it makes vs. its low risk.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: Please see comment 7

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch is very small and has a test, and it only affects @shortcuts in the urlbar

String changes made/needed: None
Attachment #9026582 - Flags: approval-mozilla-beta?
I have reproduced this issue using Firefox 65.0a1 (2018.11.15) on Win 10 x64.
I can confirm this issue is fixed, I verified using Firefox 65.0a1 on Win 10 x64, Ubuntu 18.04 x64 and Mac OS X 10.13.6.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Comment on attachment 9026582 [details] [diff] [review]
Beta/64 patch

small tweak for search shortcuts, approved for 64.0b12
Attachment #9026582 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
I can confirm this issue is fixed, I verified using Firefox 64.0b12 on Win 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.13.6.
Flags: qe-verify+
Blocks: 1520911
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: