Closed Bug 418243 Opened 17 years ago Closed 17 years ago

Removing active autocomplete textbox breaks all autocomplete functionality

Categories

(Toolkit :: Autocomplete, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: dan, Assigned: Mardak)

References

Details

(Keywords: regression, Whiteboard: [fixes bug 404135])

Attachments

(2 files, 3 obsolete files)

User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.12pre) Gecko/20080206 Camino/1.6b3pre (like Firefox/2.0.0.12pre) Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008021804 Minefield/3.0b4pre Removing an autocomplete textbox with an active results popup breaks all autocomplete functionality browser-wide. This occurs with any autocomplete textbox, not just rich-text search history ones. It's possible to reproduce the error without the popup box being open (though still by removing the textbox), but this is the most reliable method. This has been an issue on the trunk since at least September 2006. Reproducible: Always Steps to Reproduce: 1. Install attached XPI. 2. Load chrome://autocompletebreaker/content/test.xul 3. Click top textbox, enter a letter so some autocomplete results show in the popup list, press the down arrow to select one of the results (without hitting Enter), and click outside the textbox. The top textbox should disappear due to a removeChild() call in the blur() handler. 4. Click in the second textbox. Actual Results: Error: uncaught exception: [Exception... "Component does not have requested interface arg 0 [nsIAutoCompleteController.input]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "JS frame :: chrome://global/content/bindings/autocomplete.xml :: attachController :: line 265" data: no] Autocomplete is then broken program-wide and the address bar no longer functions. Expected Results: Textbox element should be removed without error. This might be the same problem behind bug 404135.
Attached file Test case extension (obsolete) (deleted) —
Version: unspecified → Trunk
FWIW, I can prevent the error with an explicit "this.mController.input = null" before removing the node in the blur() handler.
mfinkle, this is an extdev that is having trouble getting Fx3 compat... any ideas? Is this a regression?
Try putting the remove code in a setTimeout to allow the blur on the textbox to happen. That should allow the autocomplete popup to detach: var textbox = /* the textbox to remove */; setTimeout(function() { textbox.parentNode.removeChild(textbox); }, 0);
I can only reliably avoid the error with a timeout of 500ms (on a Mac Pro), which isn't really an option for us, since we use this in our main editing interface. Even at 200ms, I get the error every other time. At 0ms I get it every time. I noticed my test case extension, while it triggers the error, isn't really correct, since the handler is getting triggered by blur events on both the internal html:input (once) and the textbox (twice). I have a clearer test case that I can provide, if that'd be helpful.
Attached file Test case extension (deleted) —
Attachment #304036 - Attachment is obsolete: true
gavin: So the input isn't removing itself so the controller still has a reference to a no-longer-is-nsIAutoCompleteInput thing? ::GetInput(..) + if (mInput && !mInput->QueryInterface(nsIAutoCompleteInput)) + SetInput(nsnull); *aInput = mInput ? Might also fix bug 426525
(In reply to comment #7) > gavin: So the input isn't removing itself so the controller still has a > reference to a no-longer-is-nsIAutoCompleteInput thing? > > ::GetInput(..) > + if (mInput && !mInput->QueryInterface(nsIAutoCompleteInput)) > + SetInput(nsnull); > *aInput = mInput > > ? Might also fix bug 426525 > doesn't build. It breaks on the line: if (mInput && !mInput->QueryInterface(nsIAutoCompleteInput)) with error: error: expected primary-expression before ‘)’
I was just providing some pseudo code as a suggestion. There would probably be a nsresult returned by queryinterface anyway, so it would need a NS_FAILED and nsIAutoComplete would have to be the correct UUID.
Attachment #312982 - Attachment mime type: application/x-xpinstall → application/x-jar
The reporter claims this behavior was not in Firefox 2 so I am marking as a regression and nominating for blocking.
Flags: blocking1.9?
Keywords: regression
I know we're getting very close to final, so if we cannot land a fix, could we at least get a little help creating a work around? We feature this add-on on AMO, and we've bundled it in our campus edition of Firefox -- it is really important that these guys can update.
(In reply to comment #0) > > This might be the same problem behind bug 404135. > If it is the cause behind bug 404135, then the comment https://bugzilla.mozilla.org/show_bug.cgi?id=404135#c13 blames bug 306067.
(In reply to comment #7) > ::GetInput(..) > + if (mInput && !mInput->QueryInterface(nsIAutoCompleteInput)) > + SetInput(nsnull); > *aInput = mInput I tried that, and oddly enough it didn't fix the problem. I'm not sure how the QI in the getter can succeed while still throwing an NS_NOINTERFACE exception from somewhere in xpconnect code.
Attached patch v1 (obsolete) (deleted) — Splinter Review
I also see the interface being correct from the debugger. I'm not sure how to debug through xpcexception and CallMethod magic though..
Assignee: nobody → edilee
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #315048 - Flags: review?(gavin.sharp)
Alternatively.. we can just always assign mController.input = this instead of checking because the controller will short circuit if mInput is already aInput.
(In reply to comment #14) > Created an attachment (id=315048) [details] > v1 > > I also see the interface being correct from the debugger. I'm not sure how to > debug through xpcexception and CallMethod magic though.. > This patch also fixes bug 404135
Depends on: 428555
Attached patch v1.1 (obsolete) (deleted) — Splinter Review
Just remove the try/catch/if stuff and assign unconditionally.
Attachment #315048 - Attachment is obsolete: true
Attachment #315149 - Flags: review?(gavin.sharp)
Attachment #315048 - Flags: review?(gavin.sharp)
Comment on attachment 315149 [details] [diff] [review] v1.1 Please also adjust the indentation accordingly. r=me
Attachment #315149 - Flags: review?(gavin.sharp) → review+
Attached patch v1.2 (deleted) — Splinter Review
Fixes wanted-next bug 404135, which has a litmus testcase already. Simple one line change to fix/workaround the problem and followup for finding the source of the problem in bug 428555.
Attachment #315149 - Attachment is obsolete: true
Attachment #315164 - Flags: approval1.9?
Comment on attachment 315164 [details] [diff] [review] v1.2 a1.9=beltzner
Attachment #315164 - Flags: approval1.9? → approval1.9+
Flags: blocking1.9? → blocking1.9+
Checking in toolkit/content/widgets/autocomplete.xml; /cvsroot/mozilla/toolkit/content/widgets/autocomplete.xml,v <-- autocomplete.xml new revision: 1.138; previous revision: 1.137 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
OS: Mac OS X → All
Hardware: Macintosh → All
Resolution: --- → FIXED
Whiteboard: [fixes bug 404135]
Target Milestone: --- → mozilla1.9
Depends on: 428685
No longer depends on: 428685
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: