Closed Bug 1554038 Opened 5 months ago Closed 5 months ago

Quantumbar intermittently stops working (TypeError: result is undefined UrlbarInput.jsm:437:9)

Categories

(Firefox :: Address Bar, defect, P1)

68 Branch
defect
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 69
Iteration:
69.2 - May 27 - Jun 9
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- unaffected
firefox68 --- verified
firefox69 --- verified

People

(Reporter: bgstandaert, Assigned: dao)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0

Actual results:

Since upgrading to v68 beta with Quantumbar enabled, the address bar will intermittently stop working (maybe 1 out of 20 times?). What happens is as follows:

  1. I'll type something in the address bar, and press tab to move down to a result.
  2. Pressing enter will do nothing - the result will still be highlighted and will not load.
  3. Every time I press enter, the following error will appear in the browser console:
    TypeError: result is undefined UrlbarInput.jsm:437:9
  4. If I close the address bar and re-open it, it will start working again.

I can't make this happen reliably, and there doesn't seem to be anything else that's obviously different between when it works and when it doesn't; if there's something you want me to test to narrow this down further, let me know and I'd be happy to try it.

Component: Untriaged → Address Bar
Summary: Quantumbar intermittently stops working → Quantumbar intermittently stops working (TypeError: result is undefined UrlbarInput.jsm:437:9)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1

index must be this._queryContext.results.length because otherwise getResult would throw: https://searchfox.org/mozilla-central/rev/4606c7974a68cab416c038acaedcae49eed93822/browser/components/urlbar/UrlbarView.jsm#123

Which is a separate problem: that index check should be checking >= length, not > length...

(I also checked whether we delete any items from the results array, which would be strange, and we don't.)

So the question is, why is resultIndex in pickResult too large?

Or index in getResult might not be an integer, like undefined, but I don't see how that's possible. And there would be an error like 'ReferenceError: reference to undefined property "undefined"' if that were the case.

I can reproduce it: Type something in the urlbar and wait for the query to stop (i.e., for all results appear). Then type another character and very quickly hit tab. You need to change the selection before the results are updated to reflect the new query. The newly selected result needs to be from the previous query. (Seems easiest to do by going from a query that returns history results to one that returns search suggestions.) Then hit enter.

It looks like we don't handle this case at all, where a stale result from the previous query is picked?

Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Iteration: --- → 69.2 - May 27 - Jun 9
Points: --- → 3

(In reply to Drew Willcoxon :adw from comment #5)

It looks like we don't handle this case at all, where a stale result from the previous query is picked?

We need to pass around UrlbarResults instead of indices to address this. I'm going to post a patch to fix this specific bug, then probably file a followup to further cleanup (potentially getting rid of the resultIndex attribute entirely).

Regressed by: 1523602
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7e901c73992d
Make UrlbarInput::pickResult and UrlbarController::recordSelectedResult take a UrlbarResult instead of an index and remove UrlbarView::getResult. r=adw
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
Flags: qe-verify+

Comment on attachment 9067790 [details]
Bug 1554038 - Make UrlbarInput::pickResult and UrlbarController::recordSelectedResult take a UrlbarResult instead of an index and remove UrlbarView::getResult.

Beta/Release Uplift Approval Request

  • User impact if declined: see comment 0
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: see comment 5
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): pretty straightforward refactoring
  • String changes made/needed:
Attachment #9067790 - Flags: approval-mozilla-beta?

(In reply to Dão Gottwald [::dao] from comment #6)

(In reply to Drew Willcoxon :adw from comment #5)

It looks like we don't handle this case at all, where a stale result from the previous query is picked?

We need to pass around UrlbarResults instead of indices to address this. I'm going to post a patch to fix this specific bug, then probably file a followup to further cleanup (potentially getting rid of the resultIndex attribute entirely).

For those following along, I ended up removing resultIndex here.

QA Whiteboard: [qa-triaged]

Hello,
Reproduced the issue using steps from comment 5 with Firefox 69.0a1 (20190524214959) on Windows 10 x64, macOS 10.14 and Ubuntu 18.04.
The issue is verified using Firefox 69.0a1 (20190531035909) on Windows 10x64, Ubuntu 18.04 and macOS 10.14.

OS: Unspecified → All
Hardware: Unspecified → All

Comment on attachment 9067790 [details]
Bug 1554038 - Make UrlbarInput::pickResult and UrlbarController::recordSelectedResult take a UrlbarResult instead of an index and remove UrlbarView::getResult.

quantumbar fix for 68.0b7

Attachment #9067790 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hello,
The issue is verified using Firefox 68.0b7 (20190603181408) on Windows 10x64, macOS 10.14, Ubuntu 18.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Regressions: 1563812
You need to log in before you can comment on or make changes to this bug.