Closed Bug 168281 Opened 21 years ago Closed 21 years ago
type ahead does not work with IME
Mozilla build 2002-09-09-08-trunk When I try to enter some Chinese characters using China (Taiwan) keyboard layout on the Yahoo Taiwan homepage (http://tw.yahoo.com/), nothing happens. I do not get any kind of error messages in the status bar saying word not found, etc. It's as if I never typed anything. In contrast, I get visual feedback (characters are displayed on the top left corner of the page) when I search for Simplified Chinese links with the Chinese (China) keyboard layout. An interesting problem I noticed was that although nothing seems to happen when I enter some characters, if I then click in a textfield and attempt to enter more characters inside that field the characters I entered previously suddenly appear inside the field. Delete does not work in the textfield as expected after the reappearance, as it somehow introduces more characters into the textfield.
I would really love it if someone from Sun's China Accessibilty Task Force could investigate and help us see what's going on in the code. My Chinese is a little rusty. Yao Boo Yao Ma?
If we fix this it will probably also fix bug 168286.
aaron, your chinese is really funny :)) If you want to visit us, we can teach you more funny chinese. BTW, "Yao Boo Yao Ma?" is kind of girly, "Hao Boo Hao?" is better :)
Assignee: aaronl → kyle.yuan
Oh no! No wonder it's girly, I learned it from my old girlfriend.
a primitive patch based on my investigation. Aaron, I'm sorry, I have no time to finish this work in Sept. But I think I've found the right approach to solve this problem. I wrote a to-do comment in my code, so someone can continue this work. I'll ask someone else in ourteam to finish this work.
*** Bug 168289 has been marked as a duplicate of this bug. ***
*** Bug 168286 has been marked as a duplicate of this bug. ***
Kyle, This is a great start, thanks! A couple of questions: Why these changes? popuhidden, etc. are not from nsIDOMKeyListener chromeEventHandler->RemoveEventListener(NS_LITERAL_STRING("popuphidden"), - NS_STATIC_CAST(nsIDOMEventListener*, this), + NS_STATIC_CAST(nsIDOMKeyListener*, this), PR_TRUE); + // To-Do: + // everything is ok, then need to pass the composedText to the search engine + // (I guess it would be in KeyPress()), This makes sense to me. Would these text events encompass things beyond IME, such as text pasted into document? I guess I'll have to take a look. Maybe someone can show me how to run IME on my system. + then get the caret object, call + // caret->GetCaretCoordinates(nsICaret::eIMECoordinates, selection, + // &(textEventReply->mCursorPosition), &(textEventReply->mCursorIsCollapsed), nsnull); + // to give the feedback to the IME stuff Why does the IME have a separate selection?
On second thought, we don't want to send things into KeyPress(). We'll need to create a helper method called something like AppendCharacter(PRUnichar aCharacter) Then both KeyPress() and HandleEvent() can use AppendCharacter to do their work.
Summary: type ahead does not work with Traditional Chinese IME → type ahead does not work with IME
Sorry for my ignorance, how do I make IME work? Do I need to install another language? Or is there something for programmers who want to test using a European character set?
> Why these changes? popuhidden, etc. are not from nsIDOMKeyListener ... Because I got a compiler error for ambiguous conversion from |this| to nsIDOMEventListener. Now, |this| inherited from both nsIDOMKeyListener and nsIDOMTextListener, so there are 2 paths to convert |this| to nsIDOMEventListener. But my change is temporary, there should be another better approach. > Would these text events encompass things beyond IME, > such as text pasted into document? I dont think so, but I could be wrong. > Maybe someone can show me how to run IME on my system. You can go Control Panel, open "Regional Options", check some languages, such as Chinese (Simplified) to install. Then you may have some IME for such language. Use <ctrl>+<space> to turn on IME and <ctrl>+<shift> to switch between different IMEs. > Why does the IME have a separate selection? I just copied those code from nsPlaintextEditor::SetCompositionString, it is used to set the textEventReply->mCursorPosition and textEventReply->mCursorIsCollapsed. I'm not sure how to make a selection here :( We absolutely should ask kin who is the author of nsPlaintextEditor::SetCompositionString
Guangyan - I'm curious if the strange problems you mention have anything to do with typeaheadfind. > An interesting problem I noticed was that although nothing seems to happen when > I enter some characters, if I then click in a textfield and attempt to enter > more characters inside that field the characters I entered previously suddenly > appear inside the field. Delete does not work in the textfield as expected > after the reappearance, as it somehow introduces more characters into the > textfield If you delete all the typeahead files from your install (*typeahead*) does the problem still occur?
> We'll need to create a helper method called something like > AppendCharacter(PRUnichar aCharacter) |PRUnichar aCharacter| is wrong, we can input a bunch of characters at once by using IME. so it should be nsAString. aaron, if you can complete this function, I or Simford can do the other minor works or test. > This makes sense to me. Would these text events encompass things beyond IME, > such as text pasted into document? I tested, paste text will not emit HandleText event.
Cc jfrancis for editor related IME questions.
Aaron, in response to your question in comment #12, I removed typeahead and confirmed that the problem still exists. Therefore, it is not caused by typeahead.
Guangyan -- let me know when you file a bug for the other problem. Perhaps it should be marked as a blocker to this one.
Kyle, sounds good -- I need to get bug 167921 in first - it changes so many things that it would be hard to work on this until I get done with that. I still plan on doing AppendCharacter(), and sending the dom string 1 character at a time to it. That way AppendCharacter() can also be used by KeyPress(). I'd rather do that than turn the unichar KeyPress has into a string.
Aaron, I have filed another IME bug. It's 169076 and I will mark it as a blocker to this bug.
Kyle, does this work at all? If it works I will clean it up for review.
Attachment #99251 - Attachment is obsolete: true
Yes, it *partially* works if get rid of this line: textEvent->GetInputRange(getter_AddRefs(textRangeList)); (it will cause mozilla crash, and actually unused anywhere) I have 3 chinese IME, this patch can work with one of them, that has its own candidate window. Another two will use the target place, such as a input box, as its candidate window, so you have to deal with this situation.
Does that mean it works either way, but it just doesn't have a clean area to type if the IME program doesn't provide one?
Seeking r= There is still a problem that a blank typing area is sometimes not available, with some IME's, because they expect the blank area to be drawn by Mozilla. That will have to be a separate bug.
Attachment #100524 - Attachment is obsolete: true
Assignee: kyle.yuan → aaronl
yokoyama, can you (or shanjian) r= this patch? Talk to nhotta and gary liu about what is broken.
I talked to Aaron few minutes ago and he would like me to take a look at the patch and see if we see problem with other IMEs.
aaron, I don't think this patch is ready for review, because the HandleText() is lame. Some IME will send *every* char to HandleText() when typing, and the user is able to choose another word from candidates till he/she hit the <enter>. So you must not send the char to HandleKey(), just display them somewhere, such as the status bar, do the finding work only when received the end_composition event (I can't figure out the event name exactly yet). The only IME that can work with your patch is not a popular one. You should let MS PinYin happy first :) I'll test this on Solaris later and tell you the result.
Same problem on Solaris. nsTypeAheadFind should be a nsIDOMCompositionListener too. I'm going to update my local tree and make a new patch.
this one works better - it can work with all my IME. I show the candidate char/word in HandleText() and do finding in HandleEndComposition(). aaron, this patch is just for demo, still need more polish.
sorry, my previous patch is a garbage!
Attachment #100708 - Attachment is obsolete: true
Kyle, What still needs to be cleaned up. Do you want me to do something now? In HandleEndComposition(nsIDOMEvent* aCompositionEvent) should we do mIMEString.Truncate() at the end, in case HandleEndComposition() just to be safe? Also, should we move the caret stuff from HandleText to HandleEndComposition?
Aaron: I promised you to try this patch on my machine; but I am littel tied up other project. I hope I can get to this soon. Just to let you know :)
That's okay, Kyle and I are still working on it. One of us will put up a final patch, and the other will r= it. Once it's in we can use the i18n team to QA/verify it? I'm just glad to give you and Brian the demo yesterday, so you know what's coming soon.
Type ahead doesn't work with IME in Mac OS X either as tested on 2002092408 mozilla trunk build.
OS: Windows XP → All
1) DisplayStatus() I just append an argument into this function, is there some better way to do this? 2) + DisplayStatus(PR_FALSE, nsnull, PR_FALSE, mIMEString.get()); I just show the candidate here, Do we need something more, like a prompt and show mTypeAheadBuffer + mIMEString in case of the mixing input? 3) doing mIMEString.Truncate() at the end of HandleEndComposition() is a good idea. I remember that sometimes HandleEndComposition() will be called twice, but I'm not sure. We should leave the caret stuff in HandleText(), because the IME candidate window will disappear once HandleEndComposition() get called, we don't need to give feedback to IME any more.
Let's leave the DisplayStatus() stuff the way it is for now. I'll work with the IME team to see what the long-term plan for the status line should be. If you put the mIMEString.Truncate() in a new patch, I'll give r=
ready for r= now.
The new code makes a lot more sense. It uses methods like KeyPress(), HandleChar(), HandleBackspace(), SaveFind() etc. to split things up into more digestable pieces. I also made it so that the first IME char that isn't found stops the IME typeaheadfind, so that it doesn't keep going when it obviously cannot. Seeking r=kyle
Attachment #100838 - Attachment is obsolete: true
Comment on attachment 100935 [details] [diff] [review] Reorganized typeaheadfind into smaller, more logical pieces, as per Kyle's suggestion r=kyle
Attachment #100935 - Flags: review+
Aaron, your patch also fixed some other problems beyond this IME supporting bug, such as backspace and repeatmode enhancement. Could you say something about what that problem exactly is? one comment for the patch: should we merge the GetNodeIfTypeAheadOkay() and GetShellIfTypeAheadOkay() into one function, because we always use them together?
Kyle, as I was cleaning moving the backspace code into it's own method, I decided to deal with some cases that we ignored - what backspace and find next/prev do in repeated char find.
Attachment #100935 - Attachment is obsolete: true
Comment on attachment 101080 [details] [diff] [review] Uses Kyle's suggestion: merges GetNodeIfTypeAheadOkay + GetShellIfTypeAheadOkay -> GetTargetIfTypeAheadOkay r=kyle. looks good to me.
Attachment #101080 - Flags: review+
Comment on attachment 101080 [details] [diff] [review] Uses Kyle's suggestion: merges GetNodeIfTypeAheadOkay + GetShellIfTypeAheadOkay -> GetTargetIfTypeAheadOkay strange spacing here: browserChrome->SetStatus(nsIWebBrowserChrome::STATUS_LINK, - PromiseFlatString(statusString).get()); + PromiseFlatString(statusString).get()); other than that sr=alecf
Attachment #101080 - Flags: superreview+
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Verified with trunk.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.