1325896 - [jsplugins] Implement PPB_KeyboardInputEvent_Create

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Ya-Chieh Wu (inactive)]

We need to implement PPB_KeyboardInputEvent_Create for "backspace" and "escape".

Comment 1

[Evelyn Hung]

(In reply to Ya-Chieh Wu from comment #0)
> We need to implement PPB_KeyboardInputEvent_Create for "backspace" and
> "escape".

... for erasing one character when hitting backspace key.

Comment 2

[Ya-Chieh Wu (inactive)]

(In reply to Evelyn Hung [:evelyn] from comment #1)
> (In reply to Ya-Chieh Wu from comment #0)
> > We need to implement PPB_KeyboardInputEvent_Create for "backspace" and
> > "escape".
> 
> ... for erasing one character when hitting backspace key.

Erasing one character when hitting backspace key is not the only case. This API will be called when either "backspace" or "escape" are hitting, no matter you are typing or not.

Comment 3

[Ya-Chieh Wu (inactive)]

Attachment: 0001-Implement-PPB_KeyboardInputEvent_Create.patch

By applying this patch, you can be able to erase one character in our pdf form. Now, you can open pdf form on a second tab, which doesn't have previous page[1], to test. 

Hitting "ESC" also gets called of this API but for "ESC", we need to further fix Bug 1331298.   

[1]Bug 1273472

Comment 4

[Ya-Chieh Wu (inactive)]

Attachment: Implement PPB_KeyboardInputEvent_Create

Hi Peter, 

Could you help with this patch? PPB_KeyboardInputEvent_Create is called when "ESC" or "Backspace" are pressed. 

(1) "Backspace"
Since hitting "Backspace" will cause us go back to previous page[1], we can open a second tab, which doesn't have a previous page, to test if hitting "Backspace" can erase characters. 

(2) "ESC"
"ESC" is parsed to json escape format which we haven't fully support yet. Thus, for "ESC" to work, we need to fix Bug 1331298.

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 8830588 [details] [diff] [review]
Implement PPB_KeyboardInputEvent_Create

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

::: browser/extensions/mortar/host/common/ppapi-runtime.jsm
@@ +3066,5 @@
> +      if (PP_VarType[json.character_text.type] == PP_VarType.PP_VARTYPE_STRING) {
> +        charCode = String_PP_Var.getAsJSValue(json.character_text).charCodeAt();
> +      }
> +      let keyboardEventInit = {
> +        modifiers: json.modifiers,

This is wrong, there is no modifiers property in EventModifierInit. You need to do something like: |altkey: PP_InputEvent_Modifier.PP_INPUTEVENT_MODIFIER_ALTKEY & json.modifiers,|, etc. Also, shouldn't you set code, according to the PPAPI documentation it "represents the DOM3 |code| string that corresponds to the physical key being pressed", so it seems like passing it through makes sense?

Comment 6

[Peter Van der Beken [:peterv]]

Also, I think we should ignore charCode if we can. I wonder if that makes the ESC key work without the patch from bug 1331298?

Comment 7

[Ya-Chieh Wu (inactive)]

(In reply to Peter Van der Beken [:peterv] from comment #5)
> Also, shouldn't you set code, according to the PPAPI documentation it
> "represents the DOM3 |code| string that corresponds to the physical key
> being pressed", so it seems like passing it through makes sense?

I ignore "code" because I can't find an useful "code value" passing from plugin to gecko. At this point, I only can see "Backspace" & "ESC" use this PPB_KeyboardInputEvent:Create API.  And every time either of these key is hitting, the "code" passed to us is | code":{"type":"PP_VARTYPE_UNDEFINED","padding":0,"value":{"as_bool":"PP_FALSE","as_int":0,"as_double":0,"as_id":0}} | which doesn't really contain any information.

Comment 8

[Ya-Chieh Wu (inactive)]

(In reply to Peter Van der Beken [:peterv] from comment #6)
> Also, I think we should ignore charCode if we can. I wonder if that makes
> the ESC key work without the patch from bug 1331298?

After plugin calls PPB_KeyboardInputEvent:Create, it immediately calls PPB_KeyboardInputEvent:GetCharacterText where it needs the value of |charCode| to return. I don't know if we are able to ignore charCode here.

Comment 9

[Ya-Chieh Wu (inactive)]

Attachment: Implement-PPB_KeyboardInputEvent_Create.patch

Hi peter,

I addressed the |modifiers| issue which mentioned in Comment 5. And for skipping to pass |code| into plugin and needing to use |charCode|, I put the reasons in Comment 7 and Comment 8. 

thanks.

Comment 10

[Ya-Chieh Wu (inactive)]

Attachment: Bug-1325896-Implement-PPB_KeyboardInputEvent_Create.patch

Using KeyboardEvent.location to set the location information.

Comment 11

[Peter Van der Beken [:peterv]]

Comment on attachment 8839069 [details] [diff] [review]
Bug-1325896-Implement-PPB_KeyboardInputEvent_Create.patch

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

::: browser/extensions/mortar/host/common/ppapi-runtime.jsm
@@ +3064,5 @@
> +      let instance = this.instances[json.instance];
> +      let charCode = 0;
> +      if (PP_VarType[json.character_text.type] ==
> +          PP_VarType.PP_VARTYPE_STRING) {
> +        charCode = String_PP_Var.getAsJSValue(json.character_text).charCodeAt();

Please pass in 0 to charCodeAt.

Comment 12

[Ya-Chieh Wu (inactive)]

Attachment: 0001-Bug-1325896-Implement-PPB_KeyboardInputEvent_Create..patch

Result of try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e914cc63d160b8aed08832e1f28e31369808d4a

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e578d0aa1440
Implement PPB_KeyboardInputEvent_Create. r=peterv

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e578d0aa1440