1448282 - [TSF] Composition string may be set to empty string temporarily at every update of composition string

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Fanolian]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180322220118

Steps to reproduce:

(I am sorry if I use the IME terminologies wrong)
1. Log in to https://web.whatsapp.com/.
2. In the chat area, composite some words with affected IMEs.



Actual results:

The composite string flashes when adding more characters. (Please refer to attached videos)


Expected results:

No flashes.

Note:
There is no flashes if I set dom.keyboardevent.dispatch_during_composition to false.


Affected Windows 10 IMEs I tested:
language: Chinese Traditional (Hong Kong SAR) 中文（香港特別行政區）- 
1. (Hidden) Microsoft New Changjie, New Quick
↳ Enabling these 2 IMEs requires adding a special registry entry. Please refer to Bug 1237582 comment 0 and 2 for steps of enabling them.
2. Microsoft Bopomofo.

language: Japanese
1. Microsoft IME
↳ Flash at first 2 characters are obvious; subsequent flashes are nearly non-observable.

Comment 1

[Fanolian]

(I cannot upload videos in the bug comments as of now. Attachment will be uploaded later.)

Comment 2

[Fanolian]

Attachment: New Changjie flashes in WhatsApp Web

Video showing the flashes for Microsoft New Changjie.

(There may be a bunch of spam bug reports blocking bug 1446401 incoming, thinking the report failed to submit after waiting for few minutes.
I messed up the video settings and was uploading 100MB+ videos for previous reports. I cancelled them but they may still come through later. I am sorry.)

Comment 3

[Fanolian]

Attachment: Bopomofo flashes in WhatsApp Web

Comment 4

[Fanolian]

Attachment: Japnese IME flashes in WhatsApp Web

Comment 5

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

Thank you for the report.

Do you reproduce it even if you change UA string to Chrome? (You can switch it easily with https://addons.mozilla.org/en-US/firefox/addon/user-agent-switcher-revived/ )

Comment 6

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

(Sigh, I don't have whatsapp account and I cannot install whatsapp's app to my Android device due to "doesn't support your Android version"... So, I cannot check what they do.)

Comment 7

[Fanolian]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #5)
> Thank you for the report.
> 
> Do you reproduce it even if you change UA string to Chrome? (You can switch
> it easily with
> https://addons.mozilla.org/en-US/firefox/addon/user-agent-switcher-revived/ )

I can still reproduce it for all 3 IMEs with Chrome 65 UA.

By the way, the extension you mentioned does not work on WhatsApp Web. Maybe it is due to the service worker they use (I don't know about all these web technologies...)?
I change the UA by adding general.useragent.override in about:config.

Comment 8

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

Thank you for your test but it's oddly. We followed Chrome's (and also the others') behavior. So, if they switch to handle events with UA string, it should've worked...

And I succeeded to install whatsapp application and tested on https://web.whatsapp.com/ briefly with Nightly. However, I can use Japanese IME on Win10 normally at the input field showed immediately after starting new chat. Do I misunderstand something??

Comment 9

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

I also cannot reproduce this bug with MS Pinyin nor MS Changjie.

Comment 10

[Fanolian]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #9)
> I also cannot reproduce this bug with MS Pinyin nor MS Changjie.

I cannot reproduce it with these 2 IMEs neither. it is easiest to reproduce with Microsoft Bopomofo.

For Bopomofo:
1. Focus on WhatsApp Web chat input field.
2. Use Bopomofo and switch to Chinese input mode.
3. Type <j><i><3> (ㄨㄛˇ) repeatedly to input 我我我我...

Flashing starts at the 2nd 我 for all non-committed characters in the composite string. Once the string is committed by pressing <Enter>, the committed characters do not flash on subsequent inputs.


For Japanese IME:
2. Use Japanese IME and switch to Hiragana input mode.
3. Key in <aishiteru><space> repeatedly to input 愛してる愛してる愛してる...

The flash only appears on the last group of 愛してる, not the whole sequence of 愛してる愛してる愛してる...


For New Changjie/Quick:
Simliar to Bopomofo, Type <h><q><i><space> to input 我我我我...
However these 2 IMEs are largely abandoned (and thus hidden by default by Windows). They do not support Metro apps. (non-TSF?)

Comment 11

[Fanolian]

(In reply to Fanolian from comment #10)

> For Japanese IME:
> 2. Use Japanese IME and switch to Hiragana input mode.
> 3. Key in <aishiteru><space> repeatedly to input 愛してる愛してる愛してる...
> 
> The flash only appears on the last group of 愛してる, not the whole sequence of
> 愛してる愛してる愛してる...

The flash only appears when pressing <a> after committing a group of 愛してる, i.e. the very first keypress after pressing <space>.

For Bopomofo, every keypresses trigger a flash.

Comment 12

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

Ah, I've misunderstood what you're trying to say with "flash". Now, I got what happens.

Comment 13

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2911aa4340dc24feac46b8ccaf77892380f2843d

Comment 14

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

The actual regression point is bug 1343451 but perhaps, dispatching keydown event during composition *shows* this bug visually.

Comment 15

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

Oops, not bug 1343451. TSFTextStore was changed by bug 1259692.

Comment 16

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

Perhaps, those builds don't have this bug. Could you check it with one of them?
x86: https://queue.taskcluster.net/v1/task/fkRE7q49THiyFa-sQtbJzw/runs/0/artifacts/public/build/target.zip
x86-64: https://queue.taskcluster.net/v1/task/Eqi-YPiNQb-Infse6quO2A/runs/0/artifacts/public/build/target.zip

Comment 17

[Fanolian]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #16)
> Perhaps, those builds don't have this bug. Could you check it with one of
> them?

I can confirm the bug is fixed in both builds for the mentioned IMEs.
Thank you.

Comment 18

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

Thank you very much!

Comment 19

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

Attachment: Bug 1448282 - TSFTextStore should append a pending action for dispatching keyboard event into the queue if OnUpdateComposition() is called without new range

OnUpdateComposition() may be called without new range instance by some TIPs
when they starts to modify composition string.  At this timing, TSFTextStore
should append a pending action for dispatching keyboard event into the queue.
Without this patch, OnUpdateComposition() creates incomplete pending action
for composition update and then, MaybeDispatchKeyboardEventAsProcessedByIME()
appends pending action for dispatching keyboard event from another point
immediately (e.g., from SetText()), then, finally, the caller of
MaybeDispatchKeyboardEventAsProcessedByIME() appends another pending action
for composition update with proper composition string.  Therefore, the
first pending action for composition update clears composition string before
actually updating it with new composition string.  In other words, new
pending action for dispatching keyboard event splits a pending composition
update.  So, this patch prevents the splitting.

Review commit: https://reviewboard.mozilla.org/r/231190/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231190/

Comment 20

[Makoto Kato [:m_kato]]

Comment on attachment 8962310 [details]
Bug 1448282 - TSFTextStore should append a pending action for dispatching keyboard event into the queue if OnUpdateComposition() is called without new range

https://reviewboard.mozilla.org/r/231190/#review236904

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9d67351fd04
TSFTextStore should append a pending action for dispatching keyboard event into the queue if OnUpdateComposition() is called without new range r=m_kato

Comment 22

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d9d67351fd04