Typing a url and then keyboard navigating search one-offs doesn't properly update the heuristic
Categories
(Firefox :: Address Bar, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox84 | --- | fixed |
People
(Reporter: mak, Assigned: bugzilla)
References
Details
Attachments
(2 files)
Type a url looking string like "mozilla.org", then ALT+UP multiple times to move through search buttons, the heuristic action and icon should be updated, they are not.
I found this while working on bug 1658629, but didn't fix it there because the patch would have frown too much.
It's still an edge case anyway so likely not a show-stopper.
|
Reporter | |
Updated•4 years ago
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 1•4 years ago
|
||
The scope of this bug increased significantly as I was working on it. Full details will be in my patch's commit message, but I ended up fixing a few issues to do with one-off selection, refactored UrlbarView._on_SelectedOneOffButtonChanged, and introduced a new mechanism of swapping out the heuristic result. The patch may also need input from a11y.
|
|
Assignee | |
Comment 2•4 years ago
|
||
This patch fixes a few issues:
-
When a history result was autofilled, cycling through the engine one-offs would not change its icon (as expected) and cycling through the local one-offs would change its icon, but only to the history icon. This was because UrlbarView._iconForSearchResult was only checking for the result source and wasn't checking that the result was of search type as well. I renamed this method to _iconForResult and fixed that.
-
Typing a string, alt+arrowing through the one-offs, then pressing Enter flickers the heuristic result. This is because the one-off button is deselected, so we restore the original heuristic result. However, the search mode query also fires at the same time and quickly replaces the heuristic result. I added a check to not restore the original heuristic result in this case, eliminating the flicker.
-
When a URL result is autofilled and the user alt+arrows through the one-offs, the heuristic result contains the autofilled URL. This does not reflect what happens when the user presses Enter. The result suggests that the URL might be visited, or we might enter search mode with the URL as the search string. In reality, pressing Enter enters search mode with the typed string as the search string. The confusion is made worse by the fact that clicking the heuristic result will visit the URL and not enter search mode. This is resolved by creating a new search-type UrlbarResult and swapping it in as the heuristic result. This way, the Enter and click behaviours are consistent, the result title reflects what will happen on selection, and this fixes the filed bug of replacing the heuristic action text and icon.
This patch also refactors UrlbarView._on_SelectedOneOffButtonChanged. Previously, the main loop bailed early if a local one-off was selected, thus treating local one-offs as if they were very different from engine one-offs. This new flow handles both local and engine one-offs over the course of the same loop. I think it's a bit easier to read and understand.
I checked a11y with this patch applied and things are mostly unchanged from a screen reader perspective. Currently, typing the string "moz", alt+arrowing to the first one-off, then selecting it will create announcements like: "m", "o", "z", "Selection replaced. Google (@google), button", "Selection replaced. moz, Search with Google, edit text". The same annoucements are made with this patch applied. Pressing down arrow (not alt) will announce the second result. Arrowing back up to the heuristic will announce the original autofill result, because the original autofill result has been swapped back in. I'll ping Jamie in the bug once this patch is mature.
|
|
Assignee | |
Comment 3•4 years ago
|
||
This patch fixes two issues:
-
When a history result was autofilled, cycling through the engine one-offs would not change its icon (as expected) and cycling through the local one-offs would change its icon, but only to the history icon. This was because UrlbarView._iconForSearchResult was only checking for the result source and wasn't checking that the result was of search type as well. For consistency, we no longer change the icon when local one-offs are selected (we will change it once we figure out a way to change it for engine one-offs as well). I also renamed this method to _iconForResult to prep for Part 2 of this patch.
-
Typing a string, alt+arrowing through the one-offs, then pressing Enter flickers the heuristic result. This is because the one-off button is deselected, so we restore the original heuristic result. However, the search mode query also fires at the same time and quickly replaces the heuristic result. I added a check to not restore the original heuristic result in this case, eliminating the flicker.
This patch also refactors UrlbarView._on_SelectedOneOffButtonChanged. Previously, the main loop bailed early if a local one-off was selected, thus treating local one-offs as if they were very different from engine one-offs. This new flow handles both local and engine one-offs over the course of the same loop. I think it's a bit easier to read and understand.
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 4•4 years ago
|
||
Pushed by htwyford@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/169e2f4d602d Part 1 - Refactor UrlbarView._on_SelectedOneOffButtonChange and fix one-off selection papercuts. r=adw https://hg.mozilla.org/integration/autoland/rev/da718fa047be Part 2 - Restyle URL heuristic results as search results when cycling through one-offs. r=adw
|
||
Comment 6•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/169e2f4d602d
https://hg.mozilla.org/mozilla-central/rev/da718fa047be
Description
•