Closed Bug 1310565 Opened 8 years ago Closed 8 years ago

[Mac] When a key press causes 2 or more characters, they should be inputted with keypress events rather than a set of composition events

Categories

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

Product:
Core
Component:
Widget: Cocoa
Platform:
All
macOS
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox51 + fixed
firefox52 --- fixed
firefox53 --- verified

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod, Whiteboard: tpi:+)

Attachments

(2 files)

Bug 1310565 TextInputHandler shouldn't dispatch a composition events when a key press causes 2 or more characters
58 bytes, text/x-review-board-request
m_kato
: review+
gchang
: approval-mozilla-beta+
Details
Patch for Beta
6.86 KB, patch
Details | Diff | Splinter Review 
On Mac, TextInputHandler::InsertText() creates a composition which just sends commit string to the focused editor when a key press causes 2 or more characters.

This is not so good behavior because:

1. the text input is caused by a key press, not with IME.
2. on Windows, multiple keypress events are fired in same case.

So, using same behavior with Windows is better for both users.

Sadly, GTK widget does same behavior with Mac but I have no idea to fix same issue on it due to the difference of the API design.
Comment on attachment 8802046 [details]
Bug 1310565 TextInputHandler shouldn't dispatch a composition events when a key press causes 2 or more characters

https://reviewboard.mozilla.org/r/86600/#review85894
Attachment #8802046 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/106fc7412c9f
TextInputHandler shouldn't dispatch a composition events when a key press causes 2 or more characters r=m_kato
Priority: -- → P2
Whiteboard: tpi:+
https://hg.mozilla.org/mozilla-central/rev/106fc7412c9f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8802046 [details]
Bug 1310565 TextInputHandler shouldn't dispatch a composition events when a key press causes 2 or more characters

Approval Request Comment
[Feature/regressing bug #]: This is a follow up fix of bug 1309515. For uplifting bug 1312649 before bug 1317906, applying this patch is safer and simpler.
[User impact if declined]: This is affected only for some web developers, so, if we won't uplift bug 1312649, we don't need to uplift this.
[Describe test coverage new/current, TreeHerder]: Landed on m-c and it's now in Aurora. And this tests the new behavior and bug 1312649.
[Risks and why]: Low because the new behavior is tested and there are not so many keyboard layouts which this change affects.
[String/UUID change made/needed]: Nothing.
Attachment #8802046 - Flags: approval-mozilla-beta?
Blocks: 1312649
Depends on: 1309515
Hi Masayuki, 

Could we help here with manual testing? If yes, could you please help us with some steps to reproduce and verify the bug.

Regards,
Flags: needinfo?(masayuki)
1. Load https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html
2. Change Keyboard Layout to "Arabic - PC"
3. Press Shift+G, Shift+T, Shift+B or just B

Then, check the fired events dispatched below, there should be 2 "keypress" events between "keydown" and "keyup" events of non-modifier key. With older Gecko, you see "compositionstart", "compositionupdate" and "compositionend" between "keydown" and "keyup" instead of "keypress".
Flags: needinfo?(masayuki)
> check the fired events dispatched below

s/dispatched/displayed
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #11)
> > check the fired events dispatched below
> 
> s/dispatched/displayed

Hi Masayuki,

This is a command line that I need to use? Can you please clarify this?

About comment 10:
Thanks for helping us with the steps and test case, I tested on Mac OS X 10.10 with FF Nightly 53.0a1(2016-11-20) following the steps from comment 10 and I can confirm that I see between "keyup" and "keydown" 2 "keypress". 
You said "With older Gecko", can you please advice me how to test this with an older Gecko to see the behaviour before the fix?
Flags: needinfo?(masayuki)
(In reply to ovidiu boca[:Ovidiu] from comment #12)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #11)
> > > check the fired events dispatched below
> > 
> > s/dispatched/displayed
> 
> Hi Masayuki,
> 
> This is a command line that I need to use? Can you please clarify this?

I meant "dispatched" is typo of "displayed".

> About comment 10:
> Thanks for helping us with the steps and test case, I tested on Mac OS X
> 10.10 with FF Nightly 53.0a1(2016-11-20) following the steps from comment 10
> and I can confirm that I see between "keyup" and "keydown" 2 "keypress". 
> You said "With older Gecko", can you please advice me how to test this with
> an older Gecko to see the behaviour before the fix?

Using Release or Beta, you'll see composition events between "keydown" and "keyup".
Flags: needinfo?(masayuki)
Tested on Mac OS X 10.10 with FF Nightly 53.0a1 (2016-11-21) and I can confirm the fix.
Status: RESOLVED → VERIFIED
status-firefox53: --- → verified
Comment on attachment 8802046 [details]
Bug 1310565 TextInputHandler shouldn't dispatch a composition events when a key press causes 2 or more characters

Fix an issue related to text input handling. Beta51+. Should be in 51 beta 3.
Attachment #8802046 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Needs rebasing for beta.
Flags: needinfo?(masayuki)
Attached patch Patch for Beta — — Splinter Review 
Sorry, I forgot redesigning the automated test at 52 (bug 1300937).
Flags: needinfo?(masayuki)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: