722264 - libeditor: Use EventUtils sendChar() and sendKey(), instead of calling synthesizeKey() directly

Status: NEW

(Core :: DOM: Editor, defect)

Description

[Karun Dambiec]

sendChar() and sendKey() are higher level functions. SynthesiseKey should be replaced with these. Referencing Bug 720087.

Comment 1

[Karun Dambiec]

Attachment: Patch for this bug to replace synthesizeKey with sendKey and sendChar

This patch replaces synthesiseKey in libeditor with sendChar and sendKey, except for cases where synthesisKey is needed.

Comment 2

[Karun Dambiec]

Comment on attachment 594498 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with  sendKey and sendChar

For review. I am not sure who should review this bug.

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 594498 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with  sendKey and sendChar

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

All good. Just use sendString() too.

::: editor/libeditor/html/tests/test_bug484181.html
@@ +55,1 @@
>    }

for() + sendChar() -> sendString()

::: editor/libeditor/html/tests/test_bug551704.html
@@ +62,5 @@
>  }
>  
>  function typeBCDEF_chars() {
> +  sendChar("b");
> +  sendChar("c");

multiple sendChar() -> sendString()

@@ +66,5 @@
> +  sendChar("c");
> +  sendKey("ENTER");
> +  sendChar("d");
> +  sendChar("e");
> +  sendChar("f");

multiple sendChar() -> sendString()

::: editor/libeditor/html/tests/test_contenteditable_text_input_handling.html
@@ +125,5 @@
>  
>      if (aFocus && aFocus._isFocusable) {
> +      sendChar("A");
> +      sendChar("B");
> +      sendChar("C");

multiple sendChar() -> sendString()

Comment 4

[(no longer active)]

I'd be happy to review your patch when you address Serge's comments.

Comment 5

[Karun Dambiec]

Attachment: Patch for this bug to replace synthesizeKey with sendKey and sendChar, and sendString

I have replaced multiple sequential uses of sendChar with sendString in this version of the patch.

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 596376 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with  sendKey and sendChar, and sendString

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

::: editor/libeditor/html/tests/test_bug484181.html
@@ +50,5 @@
>    sel.selectAllChildren(edit);
>    sel.collapseToEnd();
>  
>    for (var i = 0; i < str.length; ++i) {
> +    sendChar(str[i]);

You missed to use sendSring() here.

::: editor/libeditor/html/tests/test_bug592592.html
@@ +30,5 @@
>    window.getSelection().collapse(ed.firstChild, 1);
>  
>    // Press space
> +  sendChar(" ");
> +  //sendKey("SPACE");

Don't add this extra commented out code.

Comment 7

[Karun Dambiec]

Attachment: Patch for this bug to replace synthesizeKey with sendKey and sendChar, and sendString

I have fixed the problems that was raised in this patch.

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 602597 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey and sendChar, and sendString

NB: This patch misses test_bug514156.html for example. But it is a good first part nonetheless.

Comment 9

[Karun Dambiec]

(In reply to Serge Gautherie (:sgautherie) from comment #8)
> Comment on attachment 602597 [details] [diff] [review]
> 
> NB: This patch misses test_bug514156.html for example. But it is a good
> first part nonetheless.

Yes, this is missed as when I change it to sendKey and sendChar the tests for this file fails, and it appears that this file maybe dependent on synthesiseKey, but I have not yet determined why.

Comment 10

[(no longer active)]

Thanks for your patch, Karun!  Have you tested this on the try server?  If yes, then please mark the patch as checkin?.

Comment 11

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 602597
	Branch: mozilla-central => try
Patch 602597 could not be applied to mozilla-central.
abort: bad hunk #1 @@ -25,45 +25,46 @@ SimpleTest.waitForExplicitFinish();
 (46 45 46 46)

Patchset could not be applied and pushed.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.