1559179 - Keyboard navigation through Awesome Bar suggestions jumps to search providers

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Dustin L. Howett]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

1. Open a new tab.

2. Type into the address bar. In my case, I'm typing "awesome"

3. Observe the suggestions to contain:
   [history entry 1]
   [history entry 2]
   [history entry 3]
   [search suggestion 1]
   [search suggestion 2]
   [search suggestion 3]
   [search icon] [search icon] [search icon] [search icon] [search icon]

4. Use the down arrow key to navigate to history entry 3

Actual results:

When I use the down key to navigate to history entry 3, the following happens:
Focus moves to history entry 1. Focus moves to history entry 2.
Focus moves to search icon 1

This failure is intermittent, but is often results in me choosing to search for my partially-completed address input in a random search engine.

Expected results:

Keyboard navigation should work in a reasonable manner.

Comment 1

[Drew Willcoxon :adw]

I've seen this happen occassionally, too. It's almost certainly related to the incremental addition of results and/or the key buffering we do for the down arrow. To trigger it, results have to still be loading, so you have to be a little fast.

Comment 2

[Dustin L. Howett]

I can usually reproduce this regardless of how long I wait before keyboarding.

Comment 3

[Drew Willcoxon :adw]

I was able to reproduce this twice today so far, after trying. When it happens, the down arrow key skips the search suggestions for as long as the popup remains open.

Comment 4

[Drew Willcoxon :adw]

When this happens for me, queryContext.results.length is 6 even though the number of results in the popup is 10, here: https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/browser/components/urlbar/UrlbarController.jsm#272

Wasn't there another recent (fixed) bug about using out-of-date results when the popup contains results from the previous search and the new search?

Comment 5

[Drew Willcoxon :adw]

Bug 1554038 is the one I was thinking of

Comment 6

[Drew Willcoxon :adw]

Attachment: Bug 1559179 - Quantumbar: Fix keyboard navigation when stale results are present.

Comment 7

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3a9b9667ccb1cf221c605b927f7bd818af27347

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c638a6eed13d
Quantumbar: Fix keyboard navigation when stale results are present. r=dao

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c638a6eed13d

Comment 10

[Drew Willcoxon :adw]

Comment on attachment 9075250 [details]
Bug 1559179 - Quantumbar: Fix keyboard navigation when stale results are present.

Beta/Release Uplift Approval Request

• User impact if declined: Quantumbar debuts in 68, so it would be nice to fix this in 68. This bug affects users who use the keyboard to select results in the quantumbar popup.
• 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: This bug is hard to manually reproduce. The automated works similarly to the following STR fwiw. On Nightly:

1. Type "m" in the urlbar. A list of Mozilla-related sites should appear in the urlbar popup.
2. Delete/backspace the m
3. Type "mo" and quickly press the down arrow key so that the second result in the popup becomes selected. However, don't press the down arrow key so quickly that only 6 results appear in the popup; 10 results should appear.
4. Keep pressing the down arrow key to select each result, until you get to the search buttons.

Expected: You should be able to select all results in the popup. No results should be skipped.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small patch, well understood, comes with test.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Please see above
• User impact if declined: Please see above
• Fix Landed on Version: 69
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Please see above
• String or UUID changes made by this patch: None

Comment 11

[Catalin Sasca, Desktop Test Engineering [:csasca]]

I successfully reproduced the issue on Firefox Nightly 69.0a1 (2019-06-13) under Windows 10 (x64) using the STR from Comment 10.

The issue is no longer reproducible on latest Nightly 69.0a1 (2019-07-03). Tests were performed under Windows 10 (x64), Ubuntu 18.04 (x64) and macOS 10.12.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9075250 [details]
Bug 1559179 - Quantumbar: Fix keyboard navigation when stale results are present.

This is already on m-b.

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9075250 [details]
Bug 1559179 - Quantumbar: Fix keyboard navigation when stale results are present.

quantumbar fix, verified in nightly, with tests, approved for 68 rc2

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/1beed057d962

Comment 15

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/1beed057d962

Comment 16

[Dustin L. Howett]

I did reproduce this on Nightly 69.0a1 (2019-07-04) Windows x64, which is the latest available to me. It required slightly longer input than "m", "mo", but it did reproduce every time I tried for some specific awesomebar inputs. This is true regardless of how long I wait to perform keyboard selection.

Comment 17

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Hi Dustin, I tried again on Nightly 69.0a1 (2019-07-05) under Windows 10 x64 and I couldn't reproduce it with "m", "mo". Could you please tell me the exact steps on which it did reproduce to you? Thanks!

Comment 18

[Dustin L. Howett]

Attachment: Bug1559179-2.gif

Hey Catalin,
Thanks. I've attached a GIF of my repro case. I have, in my history, a few entries for a github project called Microsoft/WSL-DistroLauncher. I was attempting to navigate to it by typing in wsl-di.

When I enter wsl-di, I get a few results of interest and one search engine result above the rest (perhaps I visited it at some point). Keyboarding down through the list exhibits the same behavior as I initially reported in this bug.

Comment 19

[Drew Willcoxon :adw]

Thanks Dustin. I can't reproduce the problem normally, but I can when I introduce some artificial latency. There's another problem that my patch didn't fix. I'll file a new bug for it.

Comment 20

[Drew Willcoxon :adw]

Attachment: Bug 1559179 - Quantumbar: Fix keyboard navigation after stale results have been removed.

We need to update indices after removing stale rows.

Comment 21

[Phabricator Automation]

Comment on attachment 9076326 [details]
Bug 1559179 - Quantumbar: Fix keyboard navigation after stale results have been removed.

Revision D37142 was moved to bug 1563812. Setting attachment 9076326 [details] to obsolete.

Comment 22

[Adrian Florinescu [:aflorinescu]]

Attachment: xriihsnm.60k.zip

Given the fact that the reproduction STR for this bug on an affected build (69.0a1 20190613095633) were intermittent, I've reused a modified profile that has very long URL browsing history (check here the details: https://bugzilla.mozilla.org/show_bug.cgi?id=1540861#c13) and with that profile, I can reproduce this report reliably with the following STR without any timing prerequisite, which I further used to verify the fixes:

[Steps:]

1. Start Fx with the attached profile.
2. (baseline)In address bar type "freakinghugeurl".
3. (baseline)Using down-arrow key to navigate throughout the results.
4. Open a new tab and type "freaking": the 1st suggestion should be freaking
5. Down-arrow key and select it, afterwhich continue writing hugeurl (now the addressbar will be: freakinghugeurl)
6. Down-arrow key to navigate through the results.

[Actual Result:]

      70.0a1 20190716211651, all good: step 3 and 6 navigate through all results.
      69.0b5 20190715173502, still reproducible (the last results are skipped)
      68.0    2019-07-05, still reproducible (the last results are skipped)
      68.0esr 	2019-07-05, still reproducible (the last results are skipped)

      Environments: Ubuntu 16.04 and Mac 10.14.  
         (didn't test on Win10 since that OS was covered already and I don't think this is OS dependent)

I'm guessing, the attached profile is catching also bug 1563812? That would explain why 70 works properly and <70 still exhibit the issue.

I'm uncertain how to proceed with marking the flags for this issue, since IMO, if the above testing approach is correct bug 1563812 seems to be necessary to fully verify this.

Drew, can you take a look over the above and advise?

Comment 23

[Drew Willcoxon :adw]

Thanks for your detailed work in verifying these bugs, Adrian. I tried the profile, and I can verify that it and the STR are hitting bug 1563812, not this bug, correct.

As for setting flags, the two bugs have the same outward appearance, but they have different causes. Your profile and STR nicely verify that bug 1563812 has been fixed, since they specifically hit that bug and not this bug. For this bug, the STR in comment 10 should be able to verify, since they specifically hit this bug.

If it would help, I can make a couple of custom builds with visual cues that would help verifying both bugs. It wouldn't be much work.

Comment 24

[Adrian Florinescu [:aflorinescu]]

Got it and thanks Drew! I think custom builds would be overkill for this verification.

Related to the STR in comment 10, I didn't use them right from the start due to the fact that the STR are timing dependent and usually for verification purposes, I want, as much as possible, to exclude from the scenarios used for reproducibility/verification the ones that involve timings, because those can be easily misleading.

Given comment 23, I think that it's fine to verify this issue fixed using comment 10 STR. Therefore I rechecked the affected build 69.0a1 20190613095633 as a reproducibility baseline, then verified this issue as fixed on:

Windows 10, Ubuntu 16.04, Mac 10.14
68.0 2019-07-05
68.0.1esr 2019-07-17
69.0b5 2019-07-15