Closed
Bug 418243
Opened 17 years ago
Closed 17 years ago
Removing active autocomplete textbox breaks all autocomplete functionality
Categories
(Toolkit :: Autocomplete, defect)
Toolkit
Autocomplete
Tracking
()
RESOLVED
FIXED
mozilla1.9
People
(Reporter: dan, Assigned: Mardak)
References
Details
(Keywords: regression, Whiteboard: [fixes bug 404135])
Attachments
(2 files, 3 obsolete files)
(deleted),
application/x-jar
|
Details | |
(deleted),
patch
|
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•17 years ago
|
||
Reporter | ||
Updated•17 years ago
|
Version: unspecified → Trunk
Reporter | ||
Comment 2•17 years ago
|
||
FWIW, I can prevent the error with an explicit "this.mController.input = null" before removing the node in the blur() handler.
Comment 3•17 years ago
|
||
mfinkle, this is an extdev that is having trouble getting Fx3 compat... any ideas? Is this a regression?
Comment 4•17 years ago
|
||
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);
Reporter | ||
Comment 5•17 years ago
|
||
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.
Reporter | ||
Comment 6•17 years ago
|
||
Attachment #304036 -
Attachment is obsolete: true
Assignee | ||
Comment 7•17 years ago
|
||
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
Comment 8•17 years ago
|
||
(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 ‘)’
Assignee | ||
Comment 9•17 years ago
|
||
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.
Updated•17 years ago
|
Attachment #312982 -
Attachment mime type: application/x-xpinstall → application/x-jar
Comment 10•17 years ago
|
||
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
Comment 11•17 years ago
|
||
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.
Comment 12•17 years ago
|
||
(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.
Comment 13•17 years ago
|
||
(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.
Assignee | ||
Comment 14•17 years ago
|
||
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)
Assignee | ||
Comment 15•17 years ago
|
||
Alternatively.. we can just always assign mController.input = this instead of checking because the controller will short circuit if mInput is already aInput.
Comment 16•17 years ago
|
||
(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
Assignee | ||
Comment 17•17 years ago
|
||
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 18•17 years ago
|
||
Comment on attachment 315149 [details] [diff] [review]
v1.1
Please also adjust the indentation accordingly. r=me
Attachment #315149 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 19•17 years ago
|
||
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 20•17 years ago
|
||
Comment on attachment 315164 [details] [diff] [review]
v1.2
a1.9=beltzner
Attachment #315164 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Assignee | ||
Comment 21•17 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•