Closed Bug 168281 Opened 20 years ago Closed 20 years ago

type ahead does not work with IME

Categories

(SeaMonkey :: Find In Page, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: gliu408, Assigned: aaronlev)

References

Details

(Keywords: inputmethod, intl)

Attachments

(1 file, 7 obsolete files)

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.
Depends on: isearch
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.
Blocks: isearch
No longer depends on: isearch
Attached patch a primitive patch (obsolete) — Splinter Review
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.  
Blocks: 169076
QA Contact: sairuh → ruixu
Keywords: intl
Blocks: 161449
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
Taking bug
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.

No longer blocks: 161449
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=
Attachment #100608 - Attachment is obsolete: true
Attachment #100709 - Attachment is obsolete: true
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+
Seeking sr=!
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+
checked in
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Verified with trunk.
Status: RESOLVED → VERIFIED
Component: Keyboard: Navigation → Keyboard: Find as you Type
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.