479445 - Fennec should show intermediate text from software input panels

Status: RESOLVED DUPLICATE of bug 487016

(Firefox for Android Graveyard :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Some input panels such as the multitap, T9 extended and transcriber tool have intermediate output that the user should be able to see.  Currently we only put the final output in the text field.

Comment 1

[Doug Turner (:dougt)]

on my htc device, there is an option so that the SIP shows the intermediate text and it works pretty nice.  What is the advantage for us to be doing this over the SIP providing this functionality?

Comment 2

[Takuro Ashie [:ashie]]

Attachment: Enables IME's composition string.

Although I'm not sure it's a same problem or not, IME's intermediate strings ("composition string" in IME's word) aren't also shown because related window messages are disabled on WinCE. Because of this issue, it's hard to input certain language on certain device. For example, "EMONSTER" (Japanese localized version of HTC TyTN II) doesn't provide such option dougt mentioned so that user can't see intermidiate string on this device. Since Japanese users aften input long intnermediate strings (because of complexity of Japanese), it's a critical issue for us Japanese.

Attached patch will temporary enables composition string of IME. But currently it has a side effect. Composition string will be enabled unexpectedly while inputting also ASCII code from both software and hardware keyborads, at least on my environment. You can confirm these issues also on Japanese WinCE emulator.

  http://www.microsoft.com/downloads/details.aspx?familyid=38c46aa8-1dd7-426f-a913-4f370a65a582&displaylang=en

Since it's a variation of WinCE 6.0, debug build may not work on it. But I confirmed that release buid can be run on this emulator image.

I'll investigate this issue and post more detailed report & patch later.
If it's different with this bug, I'll post a new bug.

Comment 3

[Doug Turner (:dougt)]

Attachment: enable IME.

Comment 4

[Doug Turner (:dougt)]

Ashie, i think you are right.  we are not handling IME intermediate text very well.  Could you try the above patch?  It removes the WINCE ifdefs around IME handling.

Comment 5

[Takuro Ashie [:ashie]]

I've tested the patch on my Japanese version of HTC Touch Diamond.
It works fine, and no side effects such as I mentioned before.

BTW I found memory leaks while investigating this issue and posted a patch as another bug (488068).
Could you check it too?

Comment 6

[Doug Turner (:dougt)]

Comment on attachment 372617 [details] [diff] [review]
enable IME.

this is great news!

Comment 7

[Ere Maijala (slow)]

Comment on attachment 372617 [details] [diff] [review]
enable IME.

Sorry, I don't sr.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 372617 [details] [diff] [review]
enable IME.


>-
>-    NS_IF_RELEASE(gCursorImgContainer);
>-
>-    if (sIsOleInitialized) {
>-      ::OleFlushClipboard();
>-      ::OleUninitialize();
>-      sIsOleInitialized = FALSE;
>-    }
>-  }
>-
>+  }
>+#ifndef WINCE
>+  NS_IF_RELEASE(gCursorImgContainer);
>+  
>+  if (sIsOleInitialized) {
>+    ::OleFlushClipboard();
>+    ::OleUninitialize();
>+    sIsOleInitialized = FALSE;
>+  }
>+  

Let's not change the code for desktop.  It looks like this should be

#ifdef WINCE 
  }
#else
    NS_IF_RELEASE(gCursorImgContainer);
...

>-#ifdef WINCE
>-      // This needs to move into nsIDOMKeyEvent.idl && nsGUIEvent.h
>-    case WM_HOTKEY:
>-    {
>-      // SmartPhones has a one or two menu buttons at the
>-      // bottom of the screen.  They are dispatched via a
>-      // menu resource, rather then a hotkey.  To make
>-      // this look consistent, we have mapped this menu to
>-      // fire hotkey events.  See
>-      // http://msdn.microsoft.com/library/default.asp?url=/library/en-us/win_ce/html/pwc_TheBackButtonandOtherInterestingButtons.asp
...
>-      
>-        case VK_APP6:
>-          keybd_event(VK_F6, 0, 0, 0);
>-          keybd_event(VK_F6, 0, KEYEVENTF_KEYUP, 0);
>-          result = 0;
>-          break;
>-      }
>-    }
>-    break;
>-#endif
>-

Why are you removing this code? It seems like its unrelated to this fix and still may be useful



>+#ifndef WINCE
>     case WM_DROPFILES:
>     {
> #if 0
>       HDROP hDropInfo = (HDROP) wParam;
>       UINT nFiles = ::DragQueryFile(hDropInfo, (UINT)-1, NULL, 0);

can you delete the code in the #if 0 ... #endif while you're here

Comment 9

[Takuro Ashie [:ashie]]

Attachment: Revised patch

I removed non-related fixes from Doug's patch.

Comment 10

[Doug Turner (:dougt)]

Attachment: patch

removing dead or unneeded code

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 374506 [details] [diff] [review]
patch

>-
>-    case WM_DROPFILES:
>-    {
>-#if 0
>-      HDROP hDropInfo = (HDROP) wParam;

You are changing behavior for desktop here because now this message will fall into the default case.  Either keep the case or make sure that this doesn't break things.

Comment 12

[Doug Turner (:dougt)]

Comment on attachment 374506 [details] [diff] [review]
patch

yeah, good call.

Comment 13

[Takuro Ashie [:ashie]]

The bug seems fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=487016.

Comment 15

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

Hi, I'm working hard on IME related code of Windows now. If you need to change them too, please add me to cc list of the bug. And also if you need to change the actual logic of the IME related code, please request the review to me. Thank you.