Closed Bug 1529467 Opened 6 years ago Closed 6 years ago

Keydown event for arrow keys does not occur when arrow keys are pressed during Hangul composition.

Categories

(Core :: Widget: Cocoa, defect, P1)

All
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla67
Tracking Status
firefox66 --- verified
firefox67 --- verified

People

(Reporter: dapsdh, Assigned: masayuki)

References

Details

(Keywords: inputmethod)

Attachments

(2 files)

Attached image win_mac.png (deleted) —

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

For Windows and Mac versions, run the following tests:

  1. Access to http://www.danilatos.com/event-test/ExperimentTest.html.
  2. Type "ㄱ" and press the right arrow key.

Actual results:

In Windows version, keydown of 39 key code occurs after compositionend, but not in Mac version.

Expected results:

If you enter a key that terminates the Hangul composition in the Mac version, key down of the key must occur after the compositionend, as in previous versions and Windows versions.

This issue causes a fatal error in my app. Please let me know when it will be patched.

Status: UNCONFIRMED → NEW
Component: Untriaged → Widget: Cocoa
Ever confirmed: true
Keywords: inputmethod
OS: Unspecified → macOS
Product: Firefox → Core
Hardware: Unspecified → All
Version: 65 Branch → Trunk

Hmm, looks like that Chrome violate the standard behavior intentionally. keydown and keyup events are physical key state change events. Therefore, while there is composition string, keydown event should be dispatched before compositionend and the keyCode value should be 229 and the key value should be Process.

I'm still not sure why macOS's IME behave differently Safari vs. Chrome/Firefox. On Safari, Korean IME does not use composition string therefore, Apple does not need to do anything hacky things. However, Chrome dispatches fake keydown event after compositionend to emulate same behavior as Safari (and perhaps native TextView).

So, we need to try to fix this ASAP.

I think that this was avoided by dispatching fake keypress event in this case. However, we stopped dispatching it starting from 65 because of conforming standard behavior (keypress event shouldn't be dispatched for non-printable keys).

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Priority: -- → P1

Currently, when IME sends a command with committing composition,
TextInputHandler::HandleCommand() dispatches a fake keypress event
for making default event handler (e.g., focused editor) handle the
event. However, we stopped dispatching keypress events for
non-printable keys. Therefore, web apps cannot do that like us.

On macOS, simple conversion IMEs like Korean 2-set IME, behave
differently if active application is changed. E.g., on Safari,
some of them may never use composition string, but not so on
Chrome and Firefox. So, this is what the case we need to emulate
Safari's behavior.

Dispatching a fake keydown event for this purpose does not
conform to UI Events because keydown events should notify web
apps of physical key state changes. However, Chrome dispatches
fake keydown events intentionally. Therefore, we should follow
this hacky behavior for user experience.

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/36874ca17f84 Make TextInputHandler::HandleCommand() dispatch eKeyDown event r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9045593 [details]
Bug 1529467 - Make TextInputHandler::HandleCommand() dispatch eKeyDown event

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 968056
  • User impact if declined: Although not truly regression of bug 968056, web apps cannot know which key is pressed at committing composition with Korean IME. (Previously, web apps can know it with fake keypress event, but we need to dispatch keydown event for them even though it's invalid behavior from the point of view of UI Events spec.)
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: 1. Load https://w3c.github.io/uievents/tools/key-event-viewer.html
  1. Use Korean 2-set IME on macOS
  2. Type KeyR to input "ㄱ".
  3. Type Enter or Tab.

Check whether keydown event for the last key is dispatched after compositionend and input events.

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): We've already dispatched keypress event for same purpose. However, now, we don't dispatch keypress events for non-printable keys. Therefore, we need to follow the Chrome's behavior for web-compat.
  • String changes made/needed: None
Attachment #9045593 - Flags: approval-mozilla-beta?

Ah, I forgot a thing. If you type a character with Korean IME at first time, the "ㄱ" may be dispatched without underline. In such case, you may don't see any problem even without the patch. However, keep typing some characters with Korean IME, you'll see underlined character. After entering this mode, you'll see this bug. (This unstable behavior is caused by macOS or Korean IME, but I'm not sure the reason. On Safari, Korean IME won't create underlined text. However, on Chrome, behaves as same as on Firefox.)

Comment on attachment 9045593 [details]
Bug 1529467 - Make TextInputHandler::HandleCommand() dispatch eKeyDown event

For this issue, let's uplift to beta and verify in beta.
Should fix some webcompat issues with korean language editing.

Attachment #9045593 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [qa-triaged]

Verified - fixed on latest Nightly 67.0a1 (2019-02-27) (64-bit) and Firefox Beta 66.0b12 (64-bit) on MacOS 10.13.
The keydown event is also dispatched on MacOS as it can be seen on Windows.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: