Closed Bug 674353 Opened 9 years ago Closed 9 years ago

Crash when pressing enter in location bar with Enter Selects addon enabled

Categories

(Toolkit :: Autocomplete, defect)

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: jdm, Assigned: jdm)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

STR:
1. install Enter Selects (with addon compatibility disabled for trunk)
2. add two search keywords that are very close (ie. mxr and mxrf)
3. In the location bar, type "mxr testing"
4. reposition the cursor to just after the |mxr|, then add an f
5. press enter before the autocomplete box appears

Expected:
either a google search for |mxrf testing| or an mxr search for |testing|

Actual:
crash
Attached file Backtrace
So we're stuck in a loop when calling nsAutoCompleteController::OpenPopup, which triggers the load event in EnterSelects (presumably, since mInput->SetPopupOpen goes directly into the xpconnect machinery and we end up interpreting from enterSelects.xul:46). This code ends up calling gURLBar.controller.handleEnter, which calls nsAutoCompleteController::PostSearchCleanup, which tries to call OpenPopup again, and we spiral downwards into oblivion.
Keywords: crash
Marco, it looks like my earlier analysis has been superseded by the changes from inline autocompletion. As such, this patch now fixes the crash for me.
Assignee: nobody → josh
So, ProcessResult checks mInput, what makes it null later, the popup actions (invalidate, open, close) in ProcessResult?
Crash Signature: [@ nsAutoCompleteController::EnterMatch] → [@ nsAutoCompleteController::EnterMatch] [@ nsAutoCompleteController::CompleteDefaultIndex]
I hoped I would be able to catch the nulling-call to SetInput in gdb, but I haven't been able to do so (too many focus problems that cause the bug not to be observed). I presume that the popup code triggers events in the Enter Selects addon, which then causes SetInput(NULL) to be called for some reason.
Crash Signature: [@ nsAutoCompleteController::EnterMatch] [@ nsAutoCompleteController::CompleteDefaultIndex] → [@ nsAutoCompleteController::EnterMatch] [@ nsAutoCompleteController::CompleteDefaultIndex] [@ nsAutoCompleteController::CompleteDefaultIndex(int)]
OS: Mac OS X → All
Comment on attachment 591457 [details] [diff] [review]
Avoid using null pointer when trying to autocomplete.

Review of attachment 591457 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -1390,5 @@
>  
>  nsresult
>  nsAutoCompleteController::CompleteDefaultIndex(PRInt32 aResultIndex)
>  {
> -  if (mDefaultIndexCompleted || mBackspaced || mSearchString.Length() == 0)

Hm, well I would have preferred some sort of comment on how we reach the mInput unbinding situation, for future reference, but if that's so hard to figure, let's begin by stopping the crashes for now.
I see in other parts of this code we have to do the same check for similar reasons.
Attachment #591457 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/863a1db98fbe
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.