Closed Bug 1653436 Opened 8 months ago Closed 7 months ago

Some characters getting clobbered if I type too fast

Categories

(Firefox :: Address Bar, defect, P1)

defect
Points:
3

Tracking

()

VERIFIED FIXED
81 Branch
Iteration:
81.1 - July 27 - Aug 09
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox78 --- unaffected
firefox79 --- unaffected
firefox80 + verified
firefox81 --- verified

People

(Reporter: overholt, Assigned: harry)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

I thought this was a dupe of bug 1652024 (or its dupes) but I'm not getting truncated results and I can still see it in the 2020-07-16 nightly build.

STR

  1. have a profile with a large PlacesDB
  2. quickly type in a few characters of a history item

Actual

  • some characters of what you type remain, sometimes with them half-completing one of your history/bookmarks (like if I type "Wennie" to go my notes for my discussions with Wennie I often end up with wfastmail.com (I used fastmail))

Expected

  • no lost characters

I'll take a look at this, thank you. It's probably a regression from bug 1648468.

Assignee: nobody → htwyford
Status: NEW → ASSIGNED

[Tracking Requested - why for this release]:

Interferes with users typing in the Urlbar. Appears to be significantly less severe/widespread than bug 1652024, but fixing it is still important.

Severity: -- → S2
Iteration: --- → 80.2 - July 13 - July 26
Points: --- → 3
Priority: -- → P1
Duplicate of this bug: 1653697
Regressed by: 1648468
Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5fbe7765a2d3
Cancel heuristic timer on cancelQuery. r=mak
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80
Duplicate of this bug: 1654133
Flags: in-testsuite+

I got a report that this bug is still an issue. Chris and Markus, I'm setting ni? since you opened duped bugs. Are you still seeing your filed issues on the latest Nightly?

Flags: needinfo?(mstange.moz)
Flags: needinfo?(cpeterson)

Yes, I still see this bug in the latest nightly ("built from https://hg.mozilla.org/mozilla-central/rev/138e7b575614cbfc1e45576a15825f51cb6e6614").

Flags: needinfo?(mstange.moz)

Thanks, I'll take a look now.

Status: RESOLVED → REOPENED
Flags: needinfo?(cpeterson)
Resolution: FIXED → ---

there is one possible mistake that I'm fixing in https://phabricator.services.mozilla.com/D84657, related to clearing this._autofillResult, but I'm honestly not sure whether it's related with chars loss at all.

That could be it. If _autofillResult is still valid when a new query is fired, Autofill will return an outdated result, which would overwrite the newer heuristic result from UrlbarProviderHeuristicFallback. Do you mind if I pull out the lines related to clearing this._autofillResult into a patch for this bug so it can land ASAP?

rebasing should not be a problem.

Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f02ec7a68ff1
Clear _autofillResult more reliably. r=mak

Can we please add a test for this case, too?

let's first check it really solves the problem for you, then we can brainstorm over a test.

Status: REOPENED → RESOLVED
Closed: 7 months ago7 months ago
Resolution: --- → FIXED

Markus, please let us know once you're confident about status of this issue.

Flags: needinfo?(mstange.moz)

This is fixed now. Thanks!

Flags: needinfo?(mstange.moz)
Blocks: 1655403

FWIW I'm still seeing this problem in the 2020-07-28 build of nightly on Windows 10. I'm happy to try debugging locally if it'll help.

Flags: needinfo?(htwyford)

(In reply to Andrew Overholt [:overholt] from comment #21)

FWIW I'm still seeing this problem in the 2020-07-28 build of nightly on Windows 10. I'm happy to try debugging locally if it'll help.

Harry told me that I'm actually likely seeing bug 1655363.

Flags: needinfo?(htwyford)

It turns out I can still see this for about: pages and also this very bug (quickly typing in "clobber" sometimes gets me "cobber", etc.).

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

I wonder if there's a relation with bug 1655034, doing more I/O may definitely mess up things here. I can't reproduce the most recent problems. Please let us know if things improve once that bug is in Nightly.

FWIW, I can reproduce this bug in 81 Nightly, but not (yet?) in 80 Beta. So perhaps this bug's fix did fix the bug in 80 Nightly but more recent change regressed it in 81 Nightly?

Which Nightly build exactly? Please test the absolutely latest one available, that's the only one that matters for now.

Duplicate of this bug: 1655936

if you read my bug (the one I just linked) it goes into a lot more detail on STR and potential factors at least from my perspective

I also just tested on the latest available nightly build: https://hg.mozilla.org/mozilla-central/rev/4c1c82402c09bd9b6a6bf57f96edc719ece2b2f3
and immediately experienced the bug within 30 seconds of startup, and I'm getting the bug repeatedly right now, not more frequently or to a higher degree of severity than the previous build but definitely not less.

The bug you filed imo is different, because it's about losing characters at the beginning of the string when switching tabs. That looks more like a problem of async tab switching and the urlbar reset happening at an unfortunate time. I don't think it's the same thing reported here.

Here the 2 problems I see reported are:

  1. losing characters at the end of the string: this was effectively a regression caused by heuristic changes we made in Firefox 80, various fixes landed, I could reproduce the bug easily, but I can't on current Nightly
  2. losing characters in the middle of the string: comment 23. My only suspects are bug 1655034, that is fixed in Nightly, and maybe autofill being more reactive while the user types and ending up clobbering something.

Could anyone reproducing this tell us which of these cases you are seing? I think Andrew is seeing 2, maybe also Chris?
Is anyone still seeing 1?

Flags: needinfo?(overholt)
Flags: needinfo?(cpeterson)
See Also: → 1656152

(In reply to Marco Bonardo [:mak] from comment #31)

  1. losing characters in the middle of the string: comment 23. My only suspects are bug 1655034, that is fixed in Nightly, and maybe autofill being more reactive while the user types and ending up clobbering something.

Could anyone reproducing this tell us which of these cases you are seing? I think Andrew is seeing 2, maybe also Chris?

Yes, I am seeing 2 with at least the 2020-07-29 build (have an update pending).

Flags: needinfo?(overholt)

Still seeing it ("about" -> "aout" is my most common reproducer) in the 2020-07-30 nightly.

It looks like UrlbarProviderTokenAliasEngines.jsm has a bug similar to the one fixed in UrlbarProviderAutofill.jsm, again I'm not sure it's "the" bug (probably not since nobody is typing @ here), but it should be fixed.

Marco found a STR in bug 1656152 comment 4 that I can reproduce: typing "wc"
very quickly. Here, Q1 is the query for "w" that has an autofill result and Q2
is the query for "wc" that does not.

The problem is produced when Q1 enters this._getAutofillResult but

  1. Q1 is cancelled while that function is awaiting and
  2. Q2 also enters this._getAutofillResult while Q1 is still inside it.

Q1 sets this._autofillResult inside this._getAutofillResult. The
instance != this.queryInstance check after Q1 returns from
this._getAutofillResult is true since Q1 is cancelled.
UrlbarProviderAutofill.isActive returns false for Q1, but this._autofillResult
is never cleared.

Q2 then finishes this._getAutofillResult without ever finding an autofill
result. However, this._autofillResult still exists when Q2 gets to this point,
since it wasn't cleared after Q1 was cancelled. The autofill result partially
overwrites the typed string.

Clearing this._autofillResult when instance != this.queryInstance is enough
to fix this bug on my machine. UrlbarProviderTokenAliasEngines has this same
issue because it also fetches a result in isActive.

(In reply to Marco Bonardo [:mak] from comment #31)

Here the 2 problems I see reported are:

  1. losing characters at the end of the string: this was effectively a regression caused by heuristic changes we made in Firefox 80, various fixes landed, I could reproduce the bug easily, but I can't on current Nightly
  2. losing characters in the middle of the string: comment 23. My only suspects are bug 1655034, that is fixed in Nightly, and maybe autofill being more reactive while the user types and ending up clobbering something.

Could anyone reproducing this tell us which of these cases you are seing? I think Andrew is seeing 2, maybe also Chris?
Is anyone still seeing 1?

When I quickly type "about:config", the actual string in the address bar is "aout:config". I can still reproduce that problem in today's Nightly 80.0a1 build 2020-07-30 but not in Beta 80.0b1. I don't know if that problem counts as your #1 or #2 (as the missing "b" character is in the middle of "about" but it disappears when "b" is at the end of "ab" while I am typing "abo").

Flags: needinfo?(cpeterson)
Attachment #9167057 - Attachment description: Bug 1653436 - Clear this.autofillResult when a query is cancelled. r?mak → Bug 1653436 - Store the queryInstance in autofillData and check it when startQuery is ready to display a result. r?mak
Attachment #9167057 - Attachment is obsolete: true
Duplicate of this bug: 1656152

mozregression confirms it as a regression from 1648468 :)

app_name: firefox
build_date: 2020-07-13 01:02:06.113000
build_type: integration
changeset: 51a157c5fe34d465fcd08a8f4d5276ae13c06f1c
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=52aae6784eb346ee86af8d1f13cdb8965c3a47be&tochange=51a157c5fe34d465fcd08a8f4d5276ae13c06f1c
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: JUJU1HxZQI2nsqkrNYnqFg

Duplicate of this bug: 1656425
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/70651770977f
Some urlbar providers should check whether the query was canceled in isActive. r=adw

Can no longer repro on autoland changeset 70651770977f. Nicely done!

Status: REOPENED → RESOLVED
Closed: 7 months ago7 months ago
Resolution: --- → FIXED

Was this actually fixed on Firefox 80? Or does the new patch need to be uplifted?

the last patch must be uplifted.

Comment on attachment 9167309 [details]
Bug 1653436 - Some urlbar providers should check whether the query was canceled in isActive. r=adw

Beta/Release Uplift Approval Request

  • User impact if declined: Autofill may happen unexpectedly, clobbering some user typed chars.
    Additionally now part of this bug is in 80 and part is not, so it'd be better to unify the code and clarify that. We should not reopen this, even if we should discover new edge cases.
  • 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: I found that if "w" autofills to www.something.com, then typing "wc" really quickly will intermittently autofill to ww.something.com instead of searching for "wc". (see bug 1648468)
    A few people could reproduce easily on their profile and may help verifying.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): I don't think it's particularly scary, we're checking if some cached data is valid before using it. We'll look into writing an automated test in bug 1655403.
  • String changes made/needed:
Attachment #9167309 - Flags: approval-mozilla-beta?
Flags: qe-verify+

If you find some cases where this still happens, please don't reopen this bug. Tracking its status is becoming problematic, so we'd prefer new bugs filed apart.
If you could reproduce the problem please test latest Nightly and let us know if it solves your problem.

QA Whiteboard: [qa-triaged]
Target Milestone: Firefox 80 → 81 Branch

Comment on attachment 9167309 [details]
Bug 1653436 - Some urlbar providers should check whether the query was canceled in isActive. r=adw

urlbar fix, approved for 80.0b4

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

Chris, are you still seeing this bug?

Flags: needinfo?(cpeterson)

(In reply to Harry Twyford [:harry] from comment #50)

Chris, are you still seeing this bug?

Nope. I can no longer reproduce this bug in 81 Nightly. 👍🏻

Flags: needinfo?(cpeterson)

Same here. I was able to reproduce this with the letters "work" 4 days ago and it is working correctly now.

Iteration: 80.2 - July 13 - July 26 → 81.1 - July 27 - Aug 09

Reproduced the issue as described in comment 47 on Nightly 81.0a1 2020-08-01 with a dirty profile.
Verified as fixed using Firefox 80 beta 7 and latest Nightly 81.0a1 2020-08-12 under Win 10 64-bit and Ubuntu 18.04 64-bit.

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