When I call "setComposition('A')", I will see a character 'A' in the input field but there is no underline. Then, if I call "setComposition('BC')", I will see 'ABC' in the input field but I expect that the previous composition string ('A' in this case) will be cleared and it shows 'BC' instead. You can try this issue by using pinyin or zhuyin IME. The regression window of gecko.git: last good build: (12/01) 0e3362fb5625eb6d98c7617b1b3019a2cc553d47 first bad build: (12/02) 668fbfecaf890207a96a19636ba8bca6d60c1823

Sorry, please ignore the regression window I mentioned above. Following is my testing environment: gaia: ee1bfd4a260e732264f8486cbf62197760de984a gecko: c5fbc5c2efc86f33591a23b7e54c6ff4f1a472e0 The last good build in my test is 017ed20347cae2cd892d7ef144e7b36cd44f0185 (11/21).

I know why I can't narrow down the regression window. When testing on builds after 11/19, it works for first time I input but will fail if I click the "backspace" to clear the composition strings and input again. So I think the last good build is b82ef1a8faa5709a9b902b8673a5e8e1415f14a9 (11/19).

Yuan, Jan, Any chance one of you know what might cause this regresssion? Thanks.

Not from the top of my head, but I can take a look on Monday.

Would it be possible this only happens on <div contenteditable>?

I looked through the keyboard gecko code and didn't find any clue. We haven't made changes about the composition since late Oct.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #5) > Would it be possible this only happens on <div contenteditable>? It's not -- I have this problem on an app "My name is," and I just checked the source code [1], simply an <textarea>. Note that I can't 100% reproduce this issue from different builds -- well, 100% with my own builds, but sometimes it works well with Geeksphone's image [2]. [1] https://marketplace.firefox.com/downloads/file/232617/my-name-is-0.1.zip [2] http://downloads.geeksphone.com/

Can we get end user STR or a reduced test case that can be used to reproduce this bug?

(In reply to Jason Smith [:jsmith] from comment #8) > Can we get end user STR or a reduced test case that can be used to reproduce > this bug? People would need to build their own phone in order to include Pinyin IME to test this out. I am filing another bug to include Pinyin IME in the eng build.

STR: 1. Go to Settings > Keyboards > Selected keyboards > Add more keyboards 2. Enable Pinyin (labeled as "拼音输入") 3. Go back to home screen and go to focus on e.me search field 4. Tap N I H A O in sequence, which means "hello" in Mandarin Chinese. 5. Tap "你好" in the selection Expected: 1. The text field shows "ni hao" with an underline on step 4 2. The text field shows "你好" on step 5 Actual: 1. The text field shows "nnini hni hani hao" with no underline on step 4 2. The text field shows "nnini hni hani hao你好" on step 5 Note: 1. e.me is just one of the examples. Input on UI tests also failed too. 2. Works in Browser pages and external test page: http://jsfiddle.net/timdream/YDGgk/ It's possible that the application is interfering with the input so we need different patches to address different causes. If so please clone this bug. Blocking: 1.4? since we would like to have 3rd-party keyboard vendors to use this API even if we don't ship our own Asian IMEs.

With further testing I fail to reproduce this bug on a newer build (Gecko/Gaia around Dec 15). I am going to ask other dogfooders; if there isn't a problem anymore let's WFM this bug. It would be nice if the API is protected by tests -- bug 932145 / bug 949961 will be good candidate for this work.

FYI: On my Keon with nightly image from http://downloads.geeksphone.com/ , the bug's still there except in browser app. I'm going to pull the latest gaia and try again later... few hours later.

I am setting qawanted, asking for a better reproduce step. Apparently users must do something else before doing what's documented in comment 10 in order to trigger this bug.

As far as I can see, the only additional thing that needs to be done is to set the keyboard to Pinyin (拼音输入). 1. Update Buri to Master M-C (1.4) BuildID: 20131223040210 2. Go to Settings > Keyboards > Selected keyboards > Add more keyboards 2. Enable Pinyin (labeled as "拼音输入") 3. Go back to home screen and go to focus on e.me search field 4. Tap on the text box to bring up the keyboard 5. Switch to the Pinyin keyboard layout via the Notification area 6. Tap N I H A O in sequence, which means "hello" in Mandarin Chinese 7. Tap "你好" in the selection Using these STR, I am able to reproduce this issue according to the actual results in comment 10. I can get this to reproduce in the Email app, Message app, and the Marketplace app as well. Environmental Variables: Device: Buri Master M-C (1.4) MOZ RIL BuildID: 20131223040210 Gaia: 5c4bb7e9dcdf6f168701508922c232a67cd8ea1f Gecko: c0f85061c7d3 Version: 29.0a1 Firmware Version: V1.2_US_20131115

Sounds like we have confirmed str. We also know this regressed on Dec 1st, which makes it a regression in the 1.3 timeframe.

Since we don't ship neither Pinyin nor 3rd-party keyboard support in 1.3, this bug is not necessary to be the blocker for v1.3. nominate this bug to 1.4.

(In reply to Ivan Tsay (:ITsay) from comment #16) > Since we don't ship neither Pinyin nor 3rd-party keyboard support in 1.3, > this bug is not necessary to be the blocker for v1.3. nominate this bug to > 1.4. How are you 100% sure this isn't going to happen in 1.3? This is a regression in the 1.3 timeframe, which implies there might be other ways to reproduce this. The Pinyin IME keyboard just so happens to be one way to reproduce this, but it only applies to 1.4+. Are there other ways this bug could be triggered? This needs more investigation on other ways to reproduce this bug.

Jason, Agree with you about there may be other ways this bug could be reproduced. At this point, it seems only Pinyin can be used to reproduce this issue. In my opinion, this bug can be renominated to v1.3 if the other ways of reproduction STR are found.

This bug makes the last test case of the inputmethod mochitest failed on mozilla-central and I marked the last case as TODO: http://mxr.mozilla.org/mozilla-central/source/dom/inputmethod/mochitest/test_basic.html?force=1#124 The mochitest could help us to reproduce the bug.

(In reply to Jason Smith [:jsmith] from comment #17) > > How are you 100% sure this isn't going to happen in 1.3? This is a > regression in the 1.3 timeframe, which implies there might be other ways to > reproduce this. The Pinyin IME keyboard just so happens to be one way to > reproduce this, but it only applies to 1.4+. > > Are there other ways this bug could be triggered? This needs more > investigation on other ways to reproduce this bug. The piece of code in question is the composition APIs. There is no other way to reproduce this bug. That said, to be on the safe side, I won't object if we want to uplift the fix to 1.3 branch once the bug is fixed.

According our tests (Jan and I), now the composition API cannot work for plain input field in various apps, but it could work in browser app - awesome bar. Kind of strange as it looks like an opposite of Bug 931746. Kanru, do you happen to know are these 2 bugs related? Thanks.

This bug is caused by Bug 845874. After back out the commit 0471905a267245523ed56212ed0f8768f9a1ca3d (https://git.mozilla.org/releases/gecko.git), the composition API works fine. I'm not sure why bug 845874 will affect the composition function. I'll look into it tomorrow. @masayuki, could you shed a light for this?

FWIW, bug 845874 was backed out of Gecko 28 because of the test failure with non-australis in bug 942250.

(In reply to Yuan Xulei [:yxl] from comment #22) > This bug is caused by Bug 845874. After back out the commit > 0471905a267245523ed56212ed0f8768f9a1ca3d > (https://git.mozilla.org/releases/gecko.git), the composition API works fine. > > I'm not sure why bug 845874 will affect the composition function. I'll look > into it tomorrow. > > @masayuki, could you shed a light for this? It's odd. I have no idea. It shouldn't be the cause of regression.

I have checked underlined or not with Japanese keyboard. o: URL bar of Browser x: homescreen search bar Gaia: 34e46e7f16f19623f652dd13bc56019603cc3316 Gecko: a01c2ee1c6a90d7641a7ad8a3fb3464e90620001 Device: KEON

Hi Xulei, that's the status of this bug?

Still have no clue why bug 845874 breaks the composition. I'm digging into the composition rendering code, but there is quite much code. I need more time to find the cause.

I logged composition events dispatched to nsIMEStateManager (http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsIMEStateManager.cpp#540) with the following code: void nsIMEStateManager::DispatchCompositionEvent(nsINode* aEventTargetNode, nsPresContext* aPresContext, WidgetEvent* aEvent, nsEventStatus* aStatus, nsDispatchingCallback* aCallBack) { // Output the message name of the event to log. char* message = nullptr; switch(aEvent->message) { case NS_COMPOSITION_START: message = "NS_COMPOSITION_START"; break; case NS_COMPOSITION_UPDATE: message = "NS_COMPOSITION_UPDATE"; break; case NS_COMPOSITION_END: message = "NS_COMPOSITION_END"; break; case NS_TEXT_TEXT: message = "NS_TEXT_TEXT"; break; } if (message) { #if defined(ANDROID) __android_log_print(ANDROID_LOG_INFO, "composition", "DispatchCompositionEvent %s\n", message); #else ... } Compared the log from b2g-desktop with that from emulator, I found that NS_COMPOSITION_START, NS_COMPOSITION_UPDATE and NS_COMPOSITION_END events were not dispatched on emulator. We only got NS_TEXT_TEXT event. Therefore we failed to start and end the composition.

This is a tentative patch and not fully tested. I'll stop to have a sleep and go on tomorrow :-)

The cause of the bug: Bug 599550 introduced |seqno| (sequence number) to discard out-of-date event arrives at the parent process (the chrome process) from the child. When we synthesize composition event in the parent process with nsDOMWindowUtils::SendCompositionEvent (http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMWindowUtils.cpp#1929) and CompositionStringSynthesizer (http://mxr.mozilla.org/mozilla-central/source/dom/base/CompositionStringSynthesizer.cpp#137), the |seqno| isn't initialized and has a random value. If such |seqno| happens to be an out-of-date value, the synthesized event is discarded. That why we randomly fails to get composition work in the parent process. However in the child process, we don't use |seqno|, so even if it is uninitialized, the composition works. Solution: I give the synthesized composition events a special |seqno| value - kLatestSeqno. If that value is detected, the parent process will skip |seqno| checks. @jchen, since the |seqno| is first introduced by you, could you help to check that the workaround of |seqno| works and has no other side effect? Thanks :)

Comment on attachment 8360187 [details] [diff] [review] Fix composition v1 patch Looks a little bit tricky to me, but I'm okay. And looks like that you are modifying almost all lines using seqno. If so, could you rename it to mSeqno? (Although, I still don't like such name, "seqno", since it's too short. I assume/guess that it's abbreviation of "sequential number".)

Comment on attachment 8360187 [details] [diff] [review] Fix composition v1 patch Review of attachment 8360187 [details] [diff] [review]: ----------------------------------------------------------------- It's been a while since I worked on this :), but I think it looks okay. Note that Firefox for Android no longer uses this code, so feel free to change it to suit B2G's needs.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #31) > Comment on attachment 8360187 [details] [diff] [review] > Fix composition v1 patch > > Looks a little bit tricky to me, but I'm okay. > > And looks like that you are modifying almost all lines using seqno. If so, > could you rename it to mSeqno? (Although, I still don't like such name, > "seqno", since it's too short. I assume/guess that it's abbreviation of > "sequential number".) Thank you, Masayuki:-) I'll update the patch and rename "seqno" member to "mSeqno". "seqno" is short for sequence number. Though abbreviation isn't recommended, we use "seqno in other classes. Maybe we can stay the same with them.

(In reply to Jim Chen [:jchen :nchen] from comment #32) > Comment on attachment 8360187 [details] [diff] [review] > Fix composition v1 patch > > Review of attachment 8360187 [details] [diff] [review]: > ----------------------------------------------------------------- > > It's been a while since I worked on this :), but I think it looks okay. Note > that Firefox for Android no longer uses this code, so feel free to change it > to suit B2G's needs. Thank you. I didn't expect your reply so quickly. Are you back to China?

Rename "seqno" member to "mSeqno". https://tbpl.mozilla.org/?tree=Try&rev=a0a5ce101063

(In reply to Yuan Xulei [:yxl] from comment #34) > (In reply to Jim Chen [:jchen :nchen] from comment #32) > > Comment on attachment 8360187 [details] [diff] [review] > > Fix composition v1 patch > > > > Review of attachment 8360187 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > It's been a while since I worked on this :), but I think it looks okay. Note > > that Firefox for Android no longer uses this code, so feel free to change it > > to suit B2G's needs. > > Thank you. I didn't expect your reply so quickly. Are you back to China? No, just working late :) I do wish to visit the China office sometime this year though.

(In reply to Jim Chen [:jchen :nchen] from comment #36) > No, just working late :) I do wish to visit the China office sometime this > year though. We're looking forward to that :-）

[Blocking Requested - why for this release]: We have now hit this for 1.4 in bug 1051228. I can't see a motivation for removing the nomination for 1.4 so I'm renominating. See the description in bug 1051228 for details.

It has been in v1.4. commit c17f701bb8d2ed2c775f31a97121ea0e2b1c70b4 Author: Yuan Xulei <xyuan@mozilla.com> Date: Wed Jan 15 09:41:39 2014 -0500 Bug 946068 - Fix composition events' synthesizing in chrome process. r=masay

(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #40) > [Blocking Requested - why for this release]: > > We have now hit this for 1.4 in bug 1051228. I can't see a motivation for > removing the nomination for 1.4 so I'm renominating. See the description in > bug 1051228 for details. This had landed before 1.4 branched off so it is already on 1.4.