Closed Bug 572800 Opened 11 years ago Closed 11 years ago
Ensure previous autocomplete query has been canceled before starting a new one (was: invalid 'in' operand this
._used Place Ids)
I've hit this a couple times with no good STR, so just filing to have on record. The first time I hit this, I had opened a new tab (browser session has been open for a while) and was trying to match a recent url in the location bar. No results showed and that error was on the console. Attempts to reproduce failed. The second time I was updating Minefield and had some other oddities happening too - 1 window has chrome correct, others didn't seem to remember toolbar customizations, some extension errors, an error from DownloadUtils.jsm. This places error was in the console several times. I tried to reproduce by restarting a few times after, but no luck. Both of these have been in the past month using Minefield.
<gavin> nsAutoCompleteController::HandleText calls stopSearch() before StartSearchTimer(), but nsAutoCompleteController::HandleKeyNavigation doesn't <gavin> I'm not sure the nsIAutocompleteSearch impl should depend on the controller doing that right, though idea is: fix the controller, but also make so that autocomplete implementations cancel previous search if the controller "forgets" to do it before starting a new search.
actually fixing the above could increase responsivity of the location bar, if we end up having 2 live queries we have a slowdown we don't really need, plus bugs that could cause having results notifications when we don't expect them.
OS: Mac OS X → All
Hardware: x86 → All
Summary: Invalid 'in' operand this._usedPlaceIds - nsPlacesAutoComplete.js → Ensure previous autocomplete query has been canceled before starting a new one (was: invalid 'in' operand this._usedPlaceIds)
something like this, not sure how much of this can be automatically tested
The test sux a bit because is somehow bound to the implementation but should touch all paths ensuring stopSearch is called before startSearch (and last test is failing before the patch, passing after, no news here). The ac code is full of special cases thus it's hard to make a nice generic test (and I fear going through b-c would be too much orange-prone).
taking for now.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Comment on attachment 452459 [details] [diff] [review] patch v1.1 You should use the public domain license block for tests per http://www.mozilla.org/MPL/license-policy.html Also, please add a test where you call the places autocomplete with startSearch twice (yes impl. dependent, but we can still test that it won't fail). r=sdwilsh
Attachment #452459 - Flags: review?(sdwilsh) → review+
fixed license, not going to add another test per discussion on IRC. To summarize a test calling twice startSearch in Places impl would not expose anything interesting since we came here through a single js warning, not an error.
Attachment #452459 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a6
You need to log in before you can comment on or make changes to this bug.