Closed Bug 1559179 Opened 7 months ago Closed 7 months ago

Keyboard navigation through Awesome Bar suggestions jumps to search providers

Categories

(Firefox :: Address Bar, defect, P2)

69 Branch
defect
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 69
Iteration:
69.4 - Jun 24 - Jul 7
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- verified
firefox67 --- unaffected
firefox68 --- verified
firefox69 --- verified

People

(Reporter: dustin, Assigned: adw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

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.

Component: Untriaged → Address Bar
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64

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.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3

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

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.

Priority: P3 → P2
OS: Windows 10 → All
Hardware: x86_64 → All

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?

Bug 1554038 is the one I was thinking of

Assignee: nobody → adw
Status: NEW → ASSIGNED
Iteration: --- → 69.4 - Jun 24 - Jul 7
Points: --- → 3
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c638a6eed13d
Quantumbar: Fix keyboard navigation when stale results are present. r=dao
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
Flags: qe-verify+
Flags: in-testsuite+

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
Attachment #9075250 - Flags: approval-mozilla-release?
Attachment #9075250 - Flags: approval-mozilla-esr68?
Attachment #9075250 - Flags: approval-mozilla-beta?
Depends on: 1563073
QA Whiteboard: [qa-triaged]

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 on attachment 9075250 [details]
Bug 1559179 - Quantumbar: Fix keyboard navigation when stale results are present.

This is already on m-b.

Attachment #9075250 - Flags: approval-mozilla-beta?

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

Attachment #9075250 - Flags: approval-mozilla-release?
Attachment #9075250 - Flags: approval-mozilla-release+
Attachment #9075250 - Flags: approval-mozilla-esr68?
Attachment #9075250 - Flags: approval-mozilla-esr68+

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.

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!

Flags: needinfo?(dustin)
Depends on: 1563731
Attached image 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.

Flags: needinfo?(dustin)

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.

Depends on: 1563812

We need to update indices after removing stale rows.

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.

Attachment #9076326 - Attachment is obsolete: true
Regressed by: 1523602
Attached file 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?

Flags: needinfo?(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.

Flags: needinfo?(adw)

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

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.