We're seeing this via several mozmill tests (see bug 1051777) Just started to fail because bug 1050683 stopped taking into consideration `browser.urlbar.delay`. With this pref set to 0 I was able to reproduce the issue and got bug 1040335 as a regressor (which enabled Unified Autocomplete). ---- Typing in the urlbar really fast (<25ms between characters) appends ">> google.com" in the urlbar. I wasn't able to reproduce this issue manually, but this is reproducible via the mozmill test. We have a testcase as attachment 8470810 [details]. To run it: 1. Get the latest environment for your system from http://mozqa.com/mozmill-env/ 2. Unzip, run the start script (or activate the environment via source bin/activate) 3. Run > mozmill -t testcase.js -b %path/to/firefox/binary%
[Tracking Requested - why for this release]:
status-firefox34: --- → affected
tracking-firefox34: --- → ?
I can reproduce by typing "go " in the urlbar, then I get go >> google.com, the trailing space seems to reproduce the bug every time. this is definitely unexpected and should be fixed soon, possibly not later than the next iteration
Summary: Unexpected in-the-middle completion in the urlbar → Unexpected in-the-middle completion in the urlbar (e.g. "go >> google.com")
so, a first problem is with search engines completion, we should never autoFill if the string contains a space. Basically before calling this._matchSearchEngineUrl() we should check this._shouldAutofill... This solves the most common case with "text/s >> searchengine". But it doesn't seem to apply to https://bugzilla.mozilla.org/show_bug.cgi?id=749923#c8 or https://bugzilla.mozilla.org/show_bug.cgi?id=749923#c9 the problem more generically is that what autoFill tries to suggest doesn't match the current input, and we hit the last else in nsAutocompleteController::CompleteValue. To avoid that we try to never suggest an autoFill value that doesn't match the current input, we failed to do so in the above case cause "go " doesn't match "google.com" but we still suggested google.com, so we ended up with " go >> google.com". The other cases may be due to us not cancelling the search fast enough. By the time autoFill returns, cancel() might have been called already, we should not return any result. I think adding an early return if (!this.pendingQuery) in _addMatch will solve this. I'll attach a patch, in case I can't finish it, someone else could easily re-use this analysis.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Marco, could you please add this to the current iteration? my other bug is pending needinfo and I have a patch basically done.
Points: --- → 5
Added to Iteration 34.2 (In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from comment #6) > Marco, could you please add this to the current iteration? my other bug is > pending needinfo and I have a patch basically done.
Iteration: --- → 34.2
Created attachment 8472843 [details] [diff] [review] patch v1 Might eventually ask Blair if Tim is busy
Attachment #8472843 - Flags: review?(ttaubert)
Blair please note you will have to update your patch (sorry for bitrotting). Tests should catch eventual issues. In case you want to steal the review, feel free to.
(In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from comment #5) > so, a first problem is with search engines completion, we should never > autoFill if the string contains a space. Possibly related to bug 767364? Unfortunately the patch in comment 8 doesn't fix bug 767364.
(In reply to Ekanan Ketunuti from comment #10) > (In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from comment #5) > > so, a first problem is with search engines completion, we should never > > autoFill if the string contains a space. > > Possibly related to bug 767364? no relation, no. that bug is also old and unrelated to unified complete.
Comment on attachment 8472843 [details] [diff] [review] patch v1 Review of attachment 8472843 [details] [diff] [review]: ----------------------------------------------------------------- Patch and tests look good, did you ensure the latter are failing without your patch applied? ::: toolkit/components/places/tests/unifiedcomplete/xpcshell.ini @@ +28,5 @@ > [test_swap_protocol.js] > [test_tabmatches.js] > [test_trimming.js] > [test_typed.js] > +[test_avoid_middle_complete.js] Should move that to the top a little to be sorted properly.
Attachment #8472843 - Flags: review?(ttaubert) → review+
(In reply to Tim Taubert [:ttaubert] from comment #13) > Patch and tests look good, did you ensure the latter are failing without > your patch applied? yep, it's born to fail! (In reply to Tim Taubert [:ttaubert] from comment #13) > Should move that to the top a little to be sorted properly. I did that locally 1 minutes after attaching, but I was too lazy to update the patch in the bug.
Target Milestone: --- → mozilla34
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox34: affected → fixed
Resolution: --- → FIXED
QA Contact: alexandra.lucinet → andrei.vaida
This is now verified fixed based on Bug 1051777 - Comment 16, per my conversation with :andrei_eftimie.
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
status-firefox34: fixed → verified
Unfortunately this bug still persists. Steps to reproduce (31.4.0): 1. Have two adressess collected in Address Book like: xxxfroxxxx yyyfroyyyy 2. Typing "fro" in TO: field and changing focus to message body leaves you with ugly search text instead of full name of the contact. Output: fro >> Full Name <email@example.com>
that's a Thunderbird bug, unrelated to this change that only affects Firefox.
Indeed, and fixed for 31.5.0 with bug 1043310.
You need to log in before you can comment on or make changes to this bug.