[Input Events] Implement InputEvent.inputType
Categories
(Core :: DOM: Events, enhancement, P2)
Tracking
()
People
(Reporter: masayuki, Assigned: masayuki)
References
(Depends on 1 open bug, )
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
Bug 1447239 - Make WPT for InputEvent.inputType of each execCommand should test each call separately
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
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
Assignee | ||
Comment 1•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fcae922cbd6565d81396d4d9c2f156de021122b4
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
I created test suite: https://d-toybox.com/studio/lib/input_event_viewer.html
Assignee | ||
Comment 3•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=60e27bdd768cf853d8e49279e323d0c734eb6c34
Assignee | ||
Comment 4•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66aa1303f396db909e058d3d6c554631e7946422
Assignee | ||
Comment 5•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=769277342456410465c26426f4220185273f8437
Assignee | ||
Comment 6•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c70cbf82a65be15928ab4949816dd02ccefdd9ff
Assignee | ||
Comment 7•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1efb152fc6452fc62fc4022ec23e34ca6570197
Assignee | ||
Comment 8•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=02da031103a252396c7d6bdfa2ad9d09ed67047e
Assignee | ||
Comment 9•5 years ago
|
||
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.
Assignee | ||
Comment 10•5 years ago
|
||
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.
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=af2f44c62b30599a09dd7050d540c0fa8821b820
Comment 12•5 years ago
|
||
(review is coming. I just need to find significant amount time for this. Sorry about delay.)
Assignee | ||
Comment 13•5 years ago
|
||
No problem. This must not be so risky. So, I think that it's okay to land it at mid of a cycle.
Assignee | ||
Comment 14•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=197b251bd099ddca4ff3d2c079d3211d4e4ab258
Assignee | ||
Comment 15•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3bf71aa1d94a997f31af537757bfe74e5f4410a
Comment 16•5 years ago
|
||
Pushed by masayuki@d-toybox.com: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14735 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Comment 19•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f8237a1e70b8 https://hg.mozilla.org/mozilla-central/rev/cd0f006ea4b3
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Congratulations and thanks for the great work, Masayuki :D
Comment 21•5 years ago
|
||
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!
Assignee | ||
Comment 22•5 years ago
|
||
(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#APIsLet me know if thiese look OK. Thanks!
Awesome! Thank you for your work!
Description
•