356007 - Crashes [@ nsFormFillController::OnTextEntered]

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

According to TB there are few crashes @  	 nsFormFillController::OnTextEntered.
Possible patch coming.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: possible patch

Bryner, since ::OnTextEntered is mainly you code, could you review.
This should fix the possible crash when ownerDocument is null or when mFocusedInput is null (if that is possible).

Other change is to set the return value to something.
That is based on this comment: 
http://lxr.mozilla.org/seamonkey/source/toolkit/components/autocomplete/public/nsIAutoCompleteInput.idl#149
but it shouldn't actually change the functionality because the return value isn't actually handled: http://lxr.mozilla.org/seamonkey/source/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp#1123

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 241703 [details] [diff] [review]
possible patch

Should not be possible for mFocusedInput to be null, but better not to crash.  Similarly, I don't know how a text input could be without an ownerDocument, but bulletproofing is fine.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

ownerDocument is *not* guaranteed to be non-null.
Basically if something keeps a reference to a node, but document is deleted, then ownerDocument is null.

Comment 4

[Martijn Wargers (dead)]

Is this something worth of the branch?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Apparently yes. There are some crashes also on 1.8.

Comment 8

[Adam Guthrie]

Smaug, is there a reason you requested approval for 1.8.1.4 and not 1.8.1.3?

Comment 9

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

(In reply to comment #8)
> Smaug, is there a reason you requested approval for 1.8.1.4 and not 1.8.1.3?

He did ask for approval1.8.1.3, see bug activity. The flag was renamed to approval1.8.1.4 in preparation for a quicker than usual 1.8.1.3 release.

Comment 11

[Daniel Veditz [:dveditz]]

This became the run-away top crash in FF2.0.0.2 (more crashes than old stand-by 0x00000000 even), and don't appear on the lists for prior versions (with admitedly small populations remaining on older versions). Was this crash tickled by the password manager changes? Seems unlikely, but I don't think 2.0.0.2 made any other form-related changes.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 241703 [details] [diff] [review]
possible patch

approved for 1.8.1.4, a=dveditz for release-drivers

Comment 14

[Daniel Veditz [:dveditz]]

(In reply to comment #11)
> Was this crash tickled by the password manager changes? Seems unlikely,
> but I don't think 2.0.0.2 made any other form-related changes.

bug 286933 was fixed in 1.8.1.2 and seems more directly relevant

Comment 15

[Andreas Morawietz]

I have those annoying crashs almost every day in forums....  when will it be fixed in firefox2 ?

Comment 16

[Martijn Wargers (dead)]

Andreas, in the next security update, you can test it yourself, see:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2.0.0.4-candidates/rc1/

Comment 18

[Jay Patel [:jay]]

v.fixed on 1.8 branch and Trunk based on the latest Talkback data.  Let's keep a close eye on topcrash reports for 2.0.0.4 after the release to make sure this is gone with no regressions.