Both XPFE and Toolkit autocomplete implementations call stopSearch even before any searches have been done. Unfortunately nsPlacesAutoComplete.js wasn't expecting this, and generates a warning and an exception, which Toolkit happens to ignore as it's written in C++ but XPFE's JS autocomplete stops working.

Editing the summary for future Bugzilla queries.

(In reply to comment #1) > *** Bug 508055 has been marked as a duplicate of this bug. *** This confirms the timeframe reported in that bug: changeset b2fd524aff54 of bug 455555, merged to m-c "by" changeset 8cff4bd2121a.

ooops...sorry!

Comment on attachment 397672 [details] [diff] [review] v1.0 r=me, thanks

Comment on attachment 397672 [details] [diff] [review] v1.0 >+ catch (e) { This stops the exception but not the warning :-(

Shouldn't you just null check _pendingQuery in _stopActiveQuery anyways, rather than wrapping the entire function call in a try/catch?

(In reply to comment #10) > Shouldn't you just null check _pendingQuery in _stopActiveQuery anyways, rather > than wrapping the entire function call in a try/catch? No, because everywhere else we call it _pendingQuery should be defined (and it'd be a logic error if it wasn't). I want it to be an error in all cases except for this one to ensure our state is correct. (In reply to comment #9) > This stops the exception but not the warning :-( On the command line? We should maybe fix that bug then, but that's not a compelling reason to not use try-catch.

(In reply to comment #11) > No, because everywhere else we call it _pendingQuery should be defined (and > it'd be a logic error if it wasn't). I want it to be an error in all cases > except for this one to ensure our state is correct. Why not just check _pendingQuery in stopQuery, then?

(In reply to comment #12) > Why not just check _pendingQuery in stopQuery, then? The original goal of pulling out the logic in _stopActiveQuery was to simplify the call sites to not have to worry about various variables. This patch doesn't care what the state is - it unconditionally drops the error if it happens.

(In reply to comment #13) > (In reply to comment #12) > > Why not just check _pendingQuery in stopQuery, then? > The original goal of pulling out the logic in _stopActiveQuery was to simplify > the call sites to not have to worry about various variables. And yet you just finished telling me how that call site was exceptional :) Let's not use try/catch to blanket over all exceptions when there's an easy way to test exactly what we need to test here.

(In reply to comment #11) > (In reply to comment #9) > > This stops the exception but not the warning :-( > On the command line? No, in the Error Console, assuming you turn all the relevant prefs on: Warning: reference to undefined property this._pendingQuery Source File: components/nsPlacesAutoComplete.js Line: 635

(In reply to comment #12) > (In reply to comment #11) > > No, because everywhere else we call it _pendingQuery should be defined (and > > it'd be a logic error if it wasn't). I want it to be an error in all cases > > except for this one to ensure our state is correct. > > Why not just check _pendingQuery in stopQuery, then? this sounds better. the comment in the patch even references a private member not used anywhere in that chunk of code, which highlights the problem gavin's pointing out. also, should fix the console spam.

Comment on attachment 399607 [details] [diff] [review] v1.1 r=me, thanks

I'm sheriff tomorrow, so I'll try to land it then.

Comment on attachment 399607 [details] [diff] [review] v1.1 asking approval, small impact change with a test.

Comment on attachment 399607 [details] [diff] [review] v1.1 We don't need approval - it's a blocker.

Doesn't work with Seamonkey "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090912 SeaMonkey/2.1a1pre" With which build is the fix at the trunk?

(In reply to comment #1) > *** Bug 508055 has been marked as a duplicate of this bug. *** The fix here may have helped SeaMonkey, though it's not obvious. I reopened that bug.

> Doesn't work with Seamonkey "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; > rv:1.9.3a1pre) Gecko/20090912 SeaMonkey/2.1a1pre" > With which build is the fix at the trunk? Confirmed. It looks like this either did not get checked into the SeaMonkey trunk builds (it also fails with a 9/13 build off of comm-central) - or the fix simply doesn't work with the trunk. With a 9/13 comm-1.9.1 everything is fine now, so the branch is okay.

Autocomplete still does not work in comm-central-trunk build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090921 SeaMonkey/2.1a1pre so this appears to be not fixed yet

Can someone change the status of this bug from resolved fixed to unresolved? Autocomplete indeed still does not work in the location bar in Seamonkey2.1a1pre Build ID:{20090924013522} And the discussion of this bug goes parallel with the discussion of bug 508055 [SeaMonkey 2.1] Location bar autocomplete doesn't work anymore. Is this how it should be?

(In reply to comment #30) > Can someone change the status of this bug from resolved fixed to unresolved? > Autocomplete indeed still does not work in the location bar in > Seamonkey2.1a1pre Build ID:{20090924013522} And the discussion of this bug goes > parallel with the discussion of bug 508055 [SeaMonkey 2.1] Location bar > autocomplete doesn't work anymore. Is this how it should be? I'm not fixing SeaMonkey code. I've fixed the toolkit issue, and this bug is resolved. Bug 508055 is tracking their issue.

This bug should remain closed. Any further work on the trunk should be addressed by bug 508055 - which has been reopened. We only need one bug tracking the problem, and it shouldn't be this one any more. (It does appear as if the problem on now the trunk is something different than what this bug fixed.)