Space character occasionally skiped when writeing in Korean IME

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
2 years ago
5 days ago

People

(Reporter: tjtncks, Assigned: masayuki)

Tracking

({inputmethod, regression})

57 Branch
mozilla58
inputmethod, regression
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57+ fixed, firefox58 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170914100122

Steps to reproduce:

write korean characters in web in some forms (like search bars in site, twitch chat, etc... and type space bar while last letter doesn't finallized (blue square cover the last letter)
P.S it happens most offen in twitch search and chat.


Actual results:

occasionally (like once in 5~10 works), the last letter doesn finnalized, but space character (U+0020, I assume) doesn't appear after it.


Expected results:

space character should appear after last latter.
(Reporter)

Updated

2 years ago
Keywords: regressionwindow-wanted
Although I cannot reproduce this using Microsoft IME (Korean) + Windows 10.  Do you use same IME?
Flags: needinfo?(tjtncks)
Component: Untriaged → Widget: Win32
Keywords: inputmethod
Product: Firefox → Core
(Reporter)

Comment 2

2 years ago
yes I using default MS IME for korean.
I captured a video of it, How I upload a attachment?
Flags: needinfo?(tjtncks)
(Reporter)

Comment 3

2 years ago
Ah, I can reproduce this on Twitter.  [Space] doesn't sometimes insert space.  But I cannot reproduce address bar and simple contenteditable sample.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

2 years ago
Yes, it doesn't happen in simple forms.
I reproduced this bug even on simple <input> element like:
https://w3c.github.io/uievents/tools/key-event-viewer.html

Type "KeyA" and type "Space" quickly.

Then, I see no white spaces between Hangul characters. At that time, I see the events in this order:

> 	type			charCode	keyCode	key	code	isComposing	data	<input>.value
> 88	compositionstart	-		-	-	-	-		''	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ '
> 87	keyup			0		32		Space	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ '
> 86	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ '
> 85	keypress		32		0		Space	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 84	keydown			0		32		Space	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 83	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 82	compositionend		-		-	-	-	-		'ㅁ'	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 81	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 80	keyup			0		65	a	KeyA	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 79	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁㅁ'
> 78	compositionupdate	-		-	-	-	-		'ㅁ'	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 77	compositionstart	-		-	-	-	-		''	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 76	keyup			0		32		Space	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 75	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 74	compositionend		-		-	-	-	-		'ㅁ'	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 73	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 72	keypress		32		0		Space	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 71	keydown			0		32		Space	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 70	keyup			0		65	a	KeyA	0		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 69	input			-		-	-	-	-		-	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ'
> 68	compositionupdate	-		-	-	-	-		'ㅁ'	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ '
> 67	compositionstart	-		-	-	-	-		''	'ㅁ ㅁ ㅁ ㅁ ㅁ ㅁ '

According to this log, Space key event may be fired in a composition and ignored. As far as I've tested, I cannot reproduce this bug in non-e10s window. So, perhaps, event order is broken in remote process.
Could be a regression of bug 1389314 since it doesn't give same priority to composition events and selectionset event as keyboard events, but still not sure since it's for DnD.
tjtncks:

Could you check if one of these builds fixes this bug?
x86: https://queue.taskcluster.net/v1/task/UCFGr3GHQGuahhafVU4Xpw/runs/0/artifacts/public/build/target.zip
x64: https://queue.taskcluster.net/v1/task/fOGr0WNIRnKc5N5hZOLR2A/runs/0/artifacts/public/build/target.zip

If you cannot reproduce this bug with these builds, I'll request to review the patch.
Flags: needinfo?(tjtncks)
Duplicate of this bug: 1403919
(Reporter)

Comment 11

2 years ago
I couldn't reproduce the bug on it.
by the way, can you make the fix landed on FF57b4~5 to? as the bug was there from the 57, the beta versions are affected, too.
Flags: needinfo?(tjtncks)
(In reply to tjtncks from comment #11)
> I couldn't reproduce the bug on it.

Thank you!

> by the way, can you make the fix landed on FF57b4~5 to? as the bug was there
> from the 57, the beta versions are affected, too.

I guess it's possible because of low risk.
Comment hidden (mozreview-request)
[Tracking Requested - why for this release]:

This is serious regression of bug 1389314. This must be really annoying for Korean users and may be so some other IME users too.
Assignee: nobody → masayuki
Blocks: 1389314
Status: NEW → ASSIGNED
status-firefox56: --- → unaffected
status-firefox57: --- → affected
status-firefox58: --- → affected
tracking-firefox57: --- → ?
Component: Widget: Win32 → Event Handling
Keywords: regressionwindow-wanted → regression
OS: Unspecified → All
Hardware: Unspecified → All

Comment 15

2 years ago
mozreview-review
Comment on attachment 8913942 [details]
Bug 1400189 - Give same priority to composition events and selectionset event as keyboard events at sending those events from the main process to a remote process

https://reviewboard.mozilla.org/r/185322/#review190284

LGTM, Thanks.
Attachment #8913942 - Flags: review?(sshih) → review+

Comment 16

2 years ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d9c1d98878cb
Give same priority to composition events and selectionset event as keyboard events at sending those events from the main process to a remote process r=stone
https://hg.mozilla.org/mozilla-central/rev/d9c1d98878cb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Hi, Masayuki
Wil you request an uplift to beta 57?
Flags: needinfo?(masayuki)
(Reporter)

Comment 19

2 years ago
(In reply to Ethan Tseng [:ethan] - 57 Regression Engineering Support from comment #18)
> Hi, Masayuki
> Wil you request an uplift to beta 57?

(Masayuki Nakano [:masayuki] (JST, +0900) from comment #12)
> (In reply to tjtncks from comment #11)
> > I couldn't reproduce the bug on it.
> 
> Thank you!
> 
> > by the way, can you make the fix landed on FF57b4~5 to? as the bug was there
> > from the 57, the beta versions are affected, too.
> 
> I guess it's possible because of low risk.

I think he will.
Comment on attachment 8913942 [details]
Bug 1400189 - Give same priority to composition events and selectionset event as keyboard events at sending those events from the main process to a remote process

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of bug 1389314.

[User impact if declined]:
Some keyboard inputs immediately after committing IME composition may be discarded in content process.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]:
No.

[List of other uplifts needed for the feature/fix]:
Nothing.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Just giving same priority as keyboard events to composition events and selection set event when sending them from TabParent to its remote process.

[String changes made/needed]:
No.
Flags: needinfo?(masayuki)
Attachment #8913942 - Flags: approval-mozilla-beta?
Comment on attachment 8913942 [details]
Bug 1400189 - Give same priority to composition events and selectionset event as keyboard events at sending those events from the main process to a remote process

Recent regression, taking it.
Should be in 57b5
Attachment #8913942 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 22

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/89d8e11182d2
status-firefox57: affected → fixed
status-firefox-esr52: --- → unaffected
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #20)
> [Is this code covered by automated tests?]:
> No.
> 
> [Has the fix been verified in Nightly?]:
> Yes.
> 
> [Needs manual test from QE? If yes, steps to reproduce]:
> No.

Setting qe-verify- based on Masayuki Nakano's assessment on manual testing needs.
Flags: qe-verify-
Duplicate of this bug: 1403512
tracking-firefox57: ? → +
Component: Event Handling → User events and focus handling
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.