393191 - Crash if browser.formfill.enable set to false [@ nsAutoCompleteController::ProcessResult]

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[John P Baker]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a8pre) Gecko/2007082205 Minefield/3.0a8pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a8pre) Gecko/2007082205 Minefield/3.0a8pre

I get an immediate crash if I try to type a character in a input if browser.formfill.enable set to false

Reproducible: Always

Steps to Reproduce:
1. Set browser.formfill.enable to false
2. Go to page with a form
3. Type into an text input
Actual Results:  
Immediate crash


There is possibly a Mozilla version of this bug 393135

Comment 1

[M** A****]

I can confirm this. Tested with a clean profile

Comment 2

[Alex Koay]

Happens here too. Clicking a focused input box has the same effect.

Comment 3

[Brian Polidoro]

Here's a crash report I got:
bp-c225b5a6-50ba-11dc-8a40-001a4bd43ed6

Currently #15 on the top crash report. And there are Mac crashes reported.  

Comment 4

[Brian Polidoro]

20070821_1656 crashes
20070821_1545 ok

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1187736300&maxdate=1187740559

Comment 5

[(not reading, please use [e-mail] instead)]

investigating, as I just touched nsAutoCompleteController::ProcessResult.

Comment 6

[(not reading, please use [e-mail] instead)]

I can reproduce this on windows as well.

aResult is null, so we crash here:

aResult->GetMatchCount(&matchCount);

Working on a fix.

>	tkautoc.dll!nsAutoCompleteController::ProcessResult(int aSearchIndex=0, nsIAutoCompleteResult * aResult=0x00000000)  Line 1179 + 0x7 bytes	C++
 	tkautoc.dll!nsAutoCompleteController::OnSearchResult(nsIAutoCompleteSearch * aSearch=0x0489af70, nsIAutoCompleteResult * aResult=0x00000000)  Line 644	C++
 	satchel.dll!nsFormFillController::StartSearch(const nsAString_internal & aSearchString={...}, const nsAString_internal & aSearchParam={...}, nsIAutoCompleteResult * aPreviousResult=0x00000000, nsIAutoCompleteObserver * aListener=0x048785e4)  Line 540	C++
 	tkautoc.dll!nsAutoCompleteController::StartSearch()  Line 1003 + 0x67 bytes	C++
 	tkautoc.dll!nsAutoCompleteController::Notify(nsITimer * timer=0x09c1c750)  Line 683	C++
 	xpcom_core.dll!nsTimerImpl::Fire()  Line 388	C++
 	xpcom_core.dll!nsTimerEvent::Run()  Line 459	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fa00)  Line 491	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00bce7b8, int mayWait=1)  Line 227 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 154 + 0xc bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 170 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=3, char * * argv=0x00bc9650, const nsXREAppData * aAppData=0x00bc9a40)  Line 3069 + 0x25 bytes	C++
 	firefox.exe!main(int argc=3, char * * argv=0x00bc9650)  Line 153 + 0x12 bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	firefox.exe!mainCRTStartup()  Line 403	C
 	kernel32.dll!7c816fd7() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	

Comment 7

[(not reading, please use [e-mail] instead)]

Attachment: extend the "if (aResult)" check to prevent crash

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 277737 [details] [diff] [review]
extend the "if (aResult)" check to prevent crash

The second check isn't needed, because result will be 0 if !aResult, and 0 isn't one of the nsIAutoCompleteResult result constants. r=me with that part of the patch removed.

It seems to me like nsFormHistory::AutoCompleteSearchshould really return a non-null "empty" result with nsIAutoCompleteResult::RESULT_NOMATCH.
Should probably file a bug on that.

Comment 9

[(not reading, please use [e-mail] instead)]

Attachment: unit test will fail (and crash) with the current code, will pass with the fix

Comment 10

[(not reading, please use [e-mail] instead)]

Attachment: as checked in

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 277740 [details] [diff] [review]
unit test will fail (and crash) with the current code, will pass with the fix

We probably want to refactor these autocomplete tests to share common code at some point, but this will do for now.

Comment 12

[(not reading, please use [e-mail] instead)]

fixed.

cvs commit: Examining src
Checking in src/nsAutoCompleteController.cpp;
/cvsroot/mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cp
p,v  <--  nsAutoCompleteController.cpp
new revision: 1.63; previous revision: 1.62
done

Comment 13

[(not reading, please use [e-mail] instead)]

unit test landed:

Checking in test_393191.js;
/cvsroot/mozilla/toolkit/components/autocomplete/tests/unit/test_393191.js,v  <-
-  test_393191.js
initial revision: 1.1
done

Comment 14

[(not reading, please use [e-mail] instead)]

spin off bugs:

#393231: 

nsFormHistory::AutoCompleteSearchshould really return a non-null "empty" result with nsIAutoCompleteResult::RESULT_NOMATCH

#393233: 

refactor the autocomplete tests to share common code

Comment 16

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010805 Minefield/3.0b3pre ID:2008010805 and the steps to reproduce from seth.

Verified fixed