193544 - Textbox crash - FF10PR1 [@ nsAutoCompleteController::HandleText] esp.JA-IME

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[noririty]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030211 Phoenix/0.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030211 Phoenix/0.5

In the middle of input to search-bar or location-bar with IME (while
transformation), Phoenix crashes when clicking HTML doc or other fields.

Reproducible: Always

Steps to Reproduce:
1. Start Phoenix with a new profile.
2. Click search-bar (search box) to change focus.
3. Turn IME on.
4. Type some button: e.g. "aiueo" (then display _underlined_ or *highlighted*
Japanese)
* Underline or highlight means in the middle of transformation.
5. Click HTML doc.
Actual Results:  
Phoenix crashes.

Expected Results:  
IME transformation should be fixed (stopped), and then focus should be changed.

WFM with mozilla(M12~1.3b). So this crash is Phoenix-specific.

Crash reported IME:
- MS-IME2002
- Justsystem Atok 15 (latest patch applied)
- skkinput

Crash reported OS:
- WindowsXP Pro.
- WindowsXP HomeEdition
- Debian/sid

Comment 1

[noririty]

I sent crash report by Quality Feedback Agent.

Incident ID:
TB17236780X

Captured At:
2003/02/17 0:34

Comment 2

[Koike Kazuhiko]

Reproduced with 20030217/Linux.

Comment 3

[noririty]

Reproduced with 20030221/WinXP.

Comment 4

[noririty]

There is a possibility that this is a duplicate of bug 184202.
Can someone attach stacktrace?

-> intl and qawanted

Comment 5

[John Morrison]

The incident ID 17236780 has expired from the database; no stack available.

Comment 6

[noririty]

Attachment: StackTrace

Comment 7

[noririty]

John:
Thank you for reply.

I successfully get stack-trace using Phoenix0.2.
http://bugzilla.mozilla.org/attachment.cgi?id=119421&action=view
Can you take a look at this? Which component made it crash?

Comment 8

[John Morrison]

There is no (or virtually no) stack information in that attachment. Can you 
just crash with a current phoenix build and submit the talkback info? Thanks.

Comment 9

[noririty]

Talkback has been disabled since Phoenix0.3
Can anyone attach significant stacktrace?

Comment 10

[KENZ (Kenji Irie)]

Attachment: full stacktrace without detail info (since this is --disable-debug build)

This is the full stack trace with nightly, i686-pc-linux-gnu 2003-04-19-08.
It seems too long for me. I think there may be some too deeply recursive calls.

Comment 11

[Masaki Katakai]

Attachment: stack trace

seems to crash in nsAutoCompleteController::HandleText().

Comment 12

[Mike Connor [:mconnor]]

can someone reproduce this with a current build and get a talkback ID?

Comment 13

[noririty]

Yes, I can still reproduce this w/20030711.
I'm trying to get some talkback IDs for I've sent,
but it seems to fail.

Comment 14

[noririty]

Bug 209600 seems to me a same crash.

-> TOPCRASH

Comment 15

[Simon Paquet [:sipaq]]

Taking QA Contact

Comment 16

[Mike Connor [:mconnor]]

*** Bug 216269 has been marked as a duplicate of this bug. ***

Comment 17

[noririty]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030827 Mozilla
Firebird/0.6.1+
Still crash. TB23124826G

Comment 18

[Erik]

*** Bug 209600 has been marked as a duplicate of this bug. ***

Comment 19

[noririty]

Changing summary and reassigning to default owner and QA.
-> Autocomplete

Comment 20

[KENZ (Kenji Irie)]

Attachment: patch to avoid crash.

For more investigation, for now, I made a patch to avoid crash by checking if
mInput is non-NULL.
This patch doesn't fix fundamental problem.

Comment 21

[Asa Dotzler [:asa]]

ben, is this bandaid worth taking?

Comment 22

[Hiro]

This crashing bug is still reproduced.
If there is no chance of fixing by 0.9 releases, can a provisional patch be
checkin in order to avoid crash?
It is very gloomy. :-(

Mozilla/5.0 (Windows; U; Windows NT 5.1; ja-JP; rv:1.7b) Gecko/20040310
Firefox/0.8.0+

Comment 23

[sheng han]

*** Bug 224226 has been marked as a duplicate of this bug. ***

Comment 24

[Ben Goodger (use ben at mozilla dot org for email)]

+ing for .9 since this is a topcrash. 

Comment 25

[Ben Goodger (use ben at mozilla dot org for email)]

Bandaid checked in, branch and trunk. Leaving open setting milestone post 1.0
for further investigation. 

Comment 26

[Jungshik Shin]

*** Bug 242522 has been marked as a duplicate of this bug. ***

Comment 27

[Jay Patel [:jay]]

I haven't seen any crashes since FF10PR1 and even current data only shows about
10 crashes for that release.  Marking WFM.

Ben:  If you feel we still need to do some more investigating, or if you have
plans for a more complete/permanent solution, please reopen.

Comment 28

[Jay Patel [:jay]]

I haven't seen any crashes since FF10PR1 and even current data only shows about
10 crashes for that release.  Marking WFM.

Ben:  If you feel we still need to do some more investigating, or if you have
plans for a more complete/permanent solution, please reopen.

Comment 29

[Hiro]

(In reply to comment #28)
> I haven't seen any crashes since FF10PR1 and even current data only shows about
> 10 crashes for that release.  Marking WFM.

See comment#20
It does not only crash with an urgent patch now.
Since the fundamental cause is not repaired, if this patch is removed, even now,
crash occur.

reopen.

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Taking.

This is simple mistake of nsFormFillController::Input.
For nsAutoCompleteController::HandleText,
we must check the mFocusedInput is not null.

I will make a new patch.

Comment 31

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch to fix.

When the control lost focus, if user is composing,
the events are fired follwing order.

1.blur
2.(endcomposition)
3.input

In currently, It assumes the input events is fired before blur event.
So, we must not call HandleText after blur.
Therefore, we should check nsIAutoCompleteControl having the control.
And in order to avoid crash, if mInput is null in HandleText, we should return.

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch to fix.

I added notice comment.

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think we should create new event that is onbeforeblur event.
In the editor, we should force composition when onbeforeblur, not onblur.
I opened bug 278802.

Comment 34

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 171607 [details] [diff] [review]
Patch to fix.

Brian:
Please review and superreview it.

Comment 35

[Brian Ryner (not reading)]

Comment on attachment 171607 [details] [diff] [review]
Patch to fix.

>--- toolkit/components/autocomplete/src/nsAutoCompleteController.cpp	2 Sep 2004 00:39:38 -0000	1.24
>+++ toolkit/components/autocomplete/src/nsAutoCompleteController.cpp	18 Jan 2005 05:47:50 -0000
>@@ -164,18 +164,22 @@ nsAutoCompleteController::StartSearch(co
> 
> NS_IMETHODIMP
> nsAutoCompleteController::HandleText(PRBool aIgnoreSelection)
> {
>   // Stop current search in case it's async.
>   StopSearch();
>   // Stop the queued up search on a timer
>   ClearSearchTimer();
>-  
>-  NS_ENSURE_TRUE(mInput, NS_ERROR_NULL_POINTER);
>+
>+  NS_ASSERTION(mInput, "Called before attach to the control or detached from the control");
>+  // Note: if this assertion raise after blur and IME end composition,

s/raise/is raised/

>+  // check mInput before calling.
>+  // See https://bugzilla.mozilla.org/show_bug.cgi?id=193544#c31
>+  NS_ENSURE_TRUE(mInput, NS_OK);

This will actually emit two separate warnings in debug builds, which is a bit
much.  Since you want the descriptive text for the error, I'd suggest:

if (!mInput) {
  NS_ERROR("Called before attaching to the control or after detaching from the
control");
  return NS_OK;
}

I agree that it would be good to make the IME text be flushed before blur,
however, this is a good fix to have anyway.  I can imagine other cases in which
it would be possible to direct text input at an unfocused text field
(context-menu paste, perhaps).

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch for check-in

Brian:
Thank you for your review and superreview.
I rewrote by comment 35.
Please check-in this patch.

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

jungshik:

Could you check-in the patch?

Comment 38

[Jungshik Shin]

patch landed into the trunk

Comment 39

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you!