The default bug view has changed. See this FAQ.

Should not send keypress event before calling interpretKeyEvents

RESOLVED FIXED in mozilla10

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

(Blocks: 1 bug, {inputmethod})

Trunk
mozilla10
x86
Mac OS X
inputmethod
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
Now, we are sending keypress event to gecko when a native keydown event is not normal letter inputting event or Control key is pressed *before* calling interpretKeyEvents. This is really wrong behavior. Because the native key event may be eaten by super class or IME.

E.g., ATOK (Japanese IME) implements "Kakutei-undo" function (Control + delete) by a tricky way. ATOK sends delete key down events to us for removing some characters which is latest committed word from ATOK. And ATOK receives the events itself via interpretKeyEvents. Then, if it needs to remove a previous character of current insertion point, it sends deleteBackward command to us. So, deleteBackward is not executed sometimes.

So, I think that we should not send keypress event before interpretKeyEvents. And we should send the keypress event from command handler instead of processKeyDownEvent.
(Assignee)

Comment 1

8 years ago
Created attachment 360962 [details] [diff] [review]
Patch v0.1 (work in progress)
(Assignee)

Updated

8 years ago
Blocks: 392254
(Assignee)

Updated

6 years ago
Depends on: 519972
(Assignee)

Updated

6 years ago
Keywords: inputmethod
(Assignee)

Updated

6 years ago
Depends on: 685073
(Assignee)

Comment 2

6 years ago
Created attachment 562994 [details] [diff] [review]
Patch Should not send keypress event before calling interpretKeyEvents

This fix is risky, we should take this ASAP and let's look for regressions.
Attachment #360962 - Attachment is obsolete: true
Attachment #562994 - Flags: review?(smichaud)
Comment on attachment 562994 [details] [diff] [review]
Patch Should not send keypress event before calling interpretKeyEvents

This sounds fine to me, though I agree that it's risky.

Let's try it and see how it works out.
Attachment #562994 - Flags: review?(smichaud) → review+
(Assignee)

Comment 4

6 years ago
(In reply to Steven Michaud from comment #3)
> Comment on attachment 562994 [details] [diff] [review] [diff] [details] [review]
> Patch Should not send keypress event before calling interpretKeyEvents
> 
> This sounds fine to me, though I agree that it's risky.
> 
> Let's try it and see how it works out.

Thank you for the quick review. And the patch depends on bug 685073's patches, would you review them too? They are simple and not risky.
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/10298b25af4e

try:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=42637a15103b
Flags: in-testsuite-
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/10298b25af4e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
(Assignee)

Comment 7

6 years ago
backedout due to random crash of mochitest-chrome.
https://hg.mozilla.org/integration/mozilla-inbound/rev/86a786598f35
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

6 years ago
probably, the cause of random crash is bug 685073's. If I cannot find the cause soon, I'll reland only this patch with minor change for the dependency.
merged backout to central.
(Assignee)

Comment 10

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/308f307cabab
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/308f307cabab
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.