[webkit-reviews] review granted: [Bug 170955] Picking an emoji via the emoji dialog (Ctrl+Cmd+Space) fires inconsistent beforeinput events. : [Attachment 319133] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 170955: Picking an emoji via the emoji dialog (Ctrl+Cmd+Space) fires
inconsistent beforeinput events.
https://bugs.webkit.org/show_bug.cgi?id=170955

Attachment 319133: Patch

https://bugs.webkit.org/attachment.cgi?id=319133&action=review




--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 319133
  --> https://bugs.webkit.org/attachment.cgi?id=319133
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319133&action=review

> LayoutTests/fast/events/before-input-prevent-insert-replacement.html:40
> +	   UIHelper.replaceTextAtRange("b", 0, 1)
> +	   .then(() => {
> +	       write(`The editor now has text content:
${editable.textContent}`);
> +	       write("(3) Inserting 'c' after 'a'...");
> +	       return UIHelper.replaceTextAtRange("c", 1, 0);
> +	   })
> +	   .then(() => {
> +	       write(`The editor now has text content:
${editable.textContent}`);
> +	       write("(4) Selecting all and preventing replacement with
'd'...");
> +	       document.execCommand("SelectAll")
> +	       return UIHelper.replaceTextAtRange("d", 0, 2);
> +	   })
> +	   .then(() => {
> +	       write(`The editor now has text content:
${editable.textContent}`);
> +	       testRunner.notifyDone();
> +	   });

We typically put then after the last line like:
(() => ~).then(() => {
})

But why don't we just wrap this in an async function so that we can just do:
await UIHelper.replaceTextAtRange("c", 1, 0);
instead?

> LayoutTests/resources/ui-helper.js:116
> +    static replaceTextAtRange(text, location, length) {
> +	   return new Promise(resolve => {
> +	       testRunner.runUIScript(`(() => {
> +		   uiController.insertText("${text}", ${location}, ${length});

It's strange to call this function replaceTextAtRange when uiController's
method is called insertText
We should stick with one or the other, and I think replaceTextAtRange is a more
descriptive of the two.