Search shortcuts should hide all one-off UI when typed

VERIFIED FIXED in Firefox 64

Status

()

P1
normal
VERIFIED FIXED
6 months ago
4 months ago

People

(Reporter: mconnor, Assigned: adw)

Tracking

(Blocks: 2 bugs)

unspecified
Firefox 65
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox64 verified, firefox65 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 months ago
Clicking a Search Shortcut and typing the equivalent keyword do not behave consistently.  The one-off UX should not be shown if we're already in a one-off mode.
(Assignee)

Updated

5 months ago
Blocks: 1496772
Component: Search → Address Bar
Priority: -- → P3
(Assignee)

Comment 1

5 months ago
So... the one-off UI should disappear and reappear as you're typing search aliases?  Like:

@b    -- one-off ui still there
@bi   -- still there
@bin  -- yep, still there
@bing -- oh hey now it's gone
*backspace*
@bin  -- there it is again
@bing -- and gone

Although now that we autofill aliases, it's more like:

@b -> @bing -- gone
*backspace*
@bin        -- back
@bing       -- gone

?
Flags: needinfo?(mverdi)
No - the one-off UI should go away once you type "@" (which should show a list of @engines) and should stay that way as long as "@" is the first character. If you backspace the "@" away the dropdown also goes away. If you then start typing a naked search term or url the dropdown with the one-off UI comes back. 

This is how it works if you click the top site search shortcut:
1. Click @amazon top site
2. @amazon gets filled in the address bar and the one-off UI is removed from the dropdown.
3. Delete some characters so you have "@amaz" for example and the one-off UI is still gone.
4. Delete everything and the dropdown closes.
5. Type something else - "telescope" for example and the one-off UI is shown again.
Flags: needinfo?(mverdi)

Comment 3

5 months ago
(In reply to Verdi [:verdi] from comment #2)
> No - the one-off UI should go away once you type "@" (which should show a
> list of @engines) and should stay that way as long as "@" is the first
> character. If you backspace the "@" away the dropdown also goes away. If you
> then start typing a naked search term or url the dropdown with the one-off
> UI comes back.

Sorry but then what happens with user defined keywords that don't begin with `@`? Like https://i.imgur.com/wvxKcvY.png
(In reply to Bruce from comment #3)
> Sorry but then what happens with user defined keywords that don't begin with
> `@`? Like https://i.imgur.com/wvxKcvY.png

I think we'll have to let those work the way they always have for now - the one-off UI will show.
(Assignee)

Updated

5 months ago
Assignee: nobody → adw
Priority: P3 → P1
(Assignee)

Updated

5 months ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

5 months ago
(In reply to Verdi [:verdi] from comment #2)

This works pretty well in practice IMO, in the patch I have.  Thumbs up!
(Assignee)

Updated

5 months ago
See Also: → bug 1502236
(Assignee)

Comment 6

5 months ago
* Get rid of the browser.urlbar.oneOffSearches pref to make this patch a little more sane. It was only supposed to be temporary; see bug 1292511.
* Disable or enable the one-offs per each new search based on whether the first char is "@". The patch does this in `onResultsAdded`, where other per-search initialization happens.
* Move the urlbar's one-off UI initialization from a popupshowing listener added in the input constructor to `openAutocompletePopup` in the popup binding. The initialization can happen internally to the popup; also, it needs to know whether someone else has initialized the one-offs first so that it doesn't (re)enable them when someone else first disabled them. That's the case when you open a new window and type "@" first thing: First `oneOffSearchesEnabled = false` gets set in `onResultsAdded`, and then the popup is opened.
* Remove the `disableOneOffButtons` option from the urlbar `search` method. It's not necessary anymore now that one-offs are automatically hidden for the only caller that uses this option (new tab with the "@engine" tiles).
* Make the `oneOffSearchesEnabled` getter return the actual status of the one-off UI instead of relying on an `_oneOffSearchesEnabled` property.

Comment 10

5 months ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0cd201908c7d
Search shortcuts should hide all one-off UI when typed r=mak,Mardak

Comment 11

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0cd201908c7d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
(Assignee)

Comment 12

5 months ago
STR for QA:

Please see comment 2 for one set of STR.  The @amazon top site is on the new tab page.

For another set of STR, please follow the STR in comment 2 again, but in step 1, instead of clicking the @amazon top site, type @amazon directly in the urlbar.  With each character of @amazon that you type, including the "@", the one-off UI should not be shown.
Flags: qe-verify+
Flags: in-testsuite+
(Assignee)

Comment 13

5 months ago
Posted 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 want this bug to make it too

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 12

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Affects only the @search shortcuts feature, covered by automated tests and manual testing, and it's early in the cycle

String changes made/needed: None
Attachment #9020966 - Flags: approval-mozilla-beta?

Comment 14

5 months ago
Hi, I can Confirm this issue as fixed in Nightly 65.0a1 (2018-10-29), I will mark this issue accordingly.
status-firefox65: fixed → verified

Comment 15

5 months ago
Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/09409bf0814ea3add30fe2a220595460d7b71993
Backport Bug 1498023 - Search shortcuts should hide all one-off UI when typed r=mak,Mardak
Comment on attachment 9020966 [details] [diff] [review]
Beta/64 patch

[Triage Comment]
Fix which complements other search shortcuts improvements shipping in Fx64. Approved for 64.0b6.
Attachment #9020966 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 17

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/3931ebbd5588
status-firefox64: --- → fixed
Verified this bug using Firefox 64.0b7 on the following OSes: Windows 7x86, Windows 10x64, Ubuntu 18.04 x64 and macOS 10.14. I have followed the STR from Comment 2.
Status: RESOLVED → VERIFIED
status-firefox64: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.