Inputmethod.sendKey always returns success, also if the key got canceled

RESOLVED FIXED in 1.3 C2/1.4 S2(17jan)

Status

Firefox OS
Gaia::Keyboard
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: janjongboom, Assigned: janjongboom)

Tracking

unspecified
1.3 C2/1.4 S2(17jan)
x86
Mac OS X
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

sendKey always succeeds, even when the keydown got preventDefault'ed. We should implement this properly.
(Assignee)

Updated

4 years ago
Assignee: nobody → janjongboom
Blocks: 951275
Created attachment 8350062 [details] [diff] [review]
952080.diff

Simple patch. If keydown gets canceled there will be no key inserted so the promise should fail.

We need this f.e. in keyboard where we do autocorrect on enter, but if the key event gets canceled that shouldn't happen.
Attachment #8350062 - Flags: review?(xyuan)
Comment on attachment 8350062 [details] [diff] [review]
952080.diff

Review of attachment 8350062 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. And we need a mochitest for this ;-)

::: dom/inputmethod/MozKeyboard.js
@@ +544,5 @@
>        case "Keyboard:SendKey:Result:OK":
>          resolver.resolve();
>          break;
> +      case "Keyboard:SendKey:Result:Error":
> +        resolver.reject();

Reject with the reason why sendKey failed.

::: dom/inputmethod/forms.js
@@ +529,1 @@
>              requestId: json.requestId

We need to return some meaningful error message to let keyboard know why it failed.
Attachment #8350062 - Flags: review?(xyuan) → review+
:janjongboom goes on quest to write test.
Created attachment 8350578 [details] [diff] [review]
Patch v2 (has tests)

Patch with rejection reason included, and a mochitest.

Try: https://tbpl.mozilla.org/?tree=Try&rev=c95c83a7c234
Attachment #8350062 - Attachment is obsolete: true
Attachment #8350578 - Flags: review+
Hi Ryan, does that try look OK? I see some intermittents and a timeout.
Flags: needinfo?(ryanvm)
All known issues, yes.
Flags: needinfo?(ryanvm)
Thanks.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c0335f9a2498
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
You need to log in before you can comment on or make changes to this bug.