1447239 - [Input Events] Implement InputEvent.inputType

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement, P2)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

InputEvent.inputType must be the most important attribute for beforeinput event since when web apps receive beforeinput event, they cannot know what will happen without inputType attribute.

There are two specs:
Level 2:
https://w3c.github.io/input-events/#dom-inputevent-inputtype
Level 1:
https://rawgit.com/w3c/input-events/v1/index.html#dom-inputevent-inputtype

According to the source code, Chrome implements Level 1.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/events/InputEvent.h?l=26-72&rcl=d4c3823079fe41cd1cbef1ffe2e8b24a173f5559

Since there are not definition of "insertFromComposition", "deleteByComposition" and "deleteCompositionText". So, perhaps, we should follow Level 1. I don't know how much stable of the specs, though.

And also see this:
https://groups.google.com/a/chromium.org/d/msg/Blink-dev/W-Q1yWW-zas/n90Mk7BnBgAJ

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I created test suite: https://d-toybox.com/studio/lib/input_event_viewer.html

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60e27bdd768cf853d8e49279e323d0c734eb6c34

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66aa1303f396db909e058d3d6c554631e7946422

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=769277342456410465c26426f4220185273f8437

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=02da031103a252396c7d6bdfa2ad9d09ed67047e

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1447239 - Make WPT for InputEvent.inputType of each execCommand should test each call separately

The WPT for InputEvent.inputType of each execCommand runs all tests in a
test function.  Therefore, even if there is an unexpected result, it won't
test other part.  Additionally, it tests execCommand's result of the DOM
tree in the contenteditable so that it's not useful for checking
InputEvent.inputType for now.

Therefore, this patch makes the test returns error for each result of
each call of execCommand.

Additionally, it sets contenteditable attribute to a <p> element which cannot
store other block elements like <ul>, <ol>, <div>, etc.  Therefore, some of
the execCommand won't work on Gecko since Gecko's editor does not create
invalid child elements as far as possible.  Therefore, this patch makes it
a <div> element.

And also adding "insertHorizontalRule", "backColor", "foreColor",
"hilightColor", "fontName", "createLink", "unlink".  inputType values for
those commands are defined by current spec.  So, if they'd be changed,
we could detect it quickly.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1447239 - Implement InputEvent.inputType

This patch implements InputType.inputType which is declared by Input Events.
The attribute has already been implemented by Chrome and Safari.  Chrome
implements Input Events Level 1, but Safari implements Input Events Level 2.
 Difference between them is only whether it supports "insertFromComposition",
"deleteByComposition" and "deleteCompositionText".  This patch makes the
level switchable with pref and takes Level 1 by default because Level 2 is
still unstable around event order with composition events.

For reducing string copy cost at dispatching "input" event, this patch
makes EditorInternalInputEvent store valid input-type as enum class,
EditorInputType and resolves it to string value when
dom::InputEvent::GetInputType() is called.  Note that the reason why
this patch names the enum class as EditorInputType is, there is InputType
enum class already for avoiding conflict the name, this appends "Editor"
prefix because "input" and "beforeinput" events are fired only when an
editor has focus.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

(review is coming. I just need to find significant amount time for this. Sorry about delay.)

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No problem. This must not be so risky. So, I think that it's okay to land it at mid of a cycle.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=197b251bd099ddca4ff3d2c079d3211d4e4ab258

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8237a1e70b8
Make WPT for InputEvent.inputType of each execCommand should test each call separately r=smaug
https://hg.mozilla.org/integration/autoland/rev/cd0f006ea4b3
Implement InputEvent.inputType r=smaug

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14735 for changes under testing/web-platform/tests

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 19

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/f8237a1e70b8
https://hg.mozilla.org/mozilla-central/rev/cd0f006ea4b3

Comment 20

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

Congratulations and thanks for the great work, Masayuki :D

Comment 21

[Chris Mills [:cmills]]

I've documented this:

Added some useful details to the inputType reference page, making sure it looked OK
https://developer.mozilla.org/en-US/docs/Web/API/InputEvent/inputType

Added the relevant support data to our browser compat data:
https://github.com/mdn/browser-compat-data/pull/3347

Added the L1 spec to our SpecData, so it can be referred to in the page. I thought this was needed because as you say above, we are basically implementing L1 spec.
https://github.com/mdn/kumascript/pull/1055

Added a note to the Fx66 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/66#APIs

Let me know if thiese look OK. Thanks!

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #21)

I've documented this:

Thank you!

Added some useful details to the inputType reference page, making sure it looked OK
https://developer.mozilla.org/en-US/docs/Web/API/InputEvent/inputType

Looks good.

Added the relevant support data to our browser compat data:
https://github.com/mdn/browser-compat-data/pull/3347

At least for now, it's true. (I mean that if we'd get web-compat reports, we'd put off to ship it.)

Added the L1 spec to our SpecData, so it can be referred to in the page. I thought this was needed because as you say above, we are basically implementing L1 spec.
https://github.com/mdn/kumascript/pull/1055

Nice.

Added a note to the Fx66 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/66#APIs

Let me know if thiese look OK. Thanks!

Awesome! Thank you for your work!