163025 – Support InputEvent.inputType for the new InputEvent spec

RESOLVED FIXED Bug 163025

Support InputEvent.inputType for the new InputEvent spec

https://bugs.webkit.org/show_bug.cgi?id=163025

Summary Support InputEvent.inputType for the new InputEvent spec

Wenson Hsieh

Reported 2016-10-06 09:59:44 PDT

Support InputEvent.inputType for the new InputEvent spec. See https://www.w3.org/TR/input-events/ for more details.

Attachments
Patch (69.77 KB, patch) 2016-10-06 20:16 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-yosemite (778.35 KB, application/zip) 2016-10-06 21:23 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews116 for mac-yosemite (1.70 MB, application/zip) 2016-10-06 21:30 PDT, Build Bot	no flags	Details
Fix DRT failures. (73.95 KB, patch) 2016-10-06 23:48 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Rebased on ToT (44.73 KB, text/plain) 2016-10-07 20:28 PDT, Wenson Hsieh	no flags	Details
Fix Windows build (48.50 KB, patch) 2016-10-07 20:40 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Refactored layout tests and added a new one. (52.75 KB, patch) 2016-10-08 11:22 PDT, Wenson Hsieh	darin: review+	Details Formatted Diff Diff
Patch for landing (51.63 KB, patch) 2016-10-09 19:39 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Wenson Hsieh

Comment 1 2016-10-06 13:51:14 PDT

<rdar://problem/28658092>

Wenson Hsieh

Comment 2 2016-10-06 20:16:23 PDT

Created attachment 290883 [details] Patch

Build Bot

Comment 3 2016-10-06 21:23:33 PDT

Comment on attachment 290883 [details] Patch Attachment 290883 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2235177 New failing tests: fast/events/before-input-prevent-paste.html fast/events/before-input-prevent-cut.html

Build Bot

Comment 4 2016-10-06 21:23:36 PDT

Created attachment 290888 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5

Build Bot

Comment 5 2016-10-06 21:30:34 PDT

Comment on attachment 290883 [details] Patch Attachment 290883 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2235180 New failing tests: fast/events/before-input-prevent-paste.html fast/events/before-input-prevent-cut.html

Build Bot

Comment 6 2016-10-06 21:30:36 PDT

Created attachment 290889 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5

Wenson Hsieh

Comment 7 2016-10-06 23:48:45 PDT

Created attachment 290905 [details] Fix DRT failures.

Wenson Hsieh

Comment 8 2016-10-07 20:28:10 PDT

Created attachment 291006 [details] Rebased on ToT

Wenson Hsieh

Comment 9 2016-10-07 20:40:47 PDT

Created attachment 291007 [details] Fix Windows build

Ryosuke Niwa

Comment 10 2016-10-07 23:58:56 PDT

Comment on attachment 291007 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=291007&action=review r- because while the implementation is pretty good, there are significant issues with the way tests are written. > Source/WebCore/editing/EditCommand.cpp:41 > +AtomicString inputTypeNameForEditingAction(EditAction action) We shouldn't really be using AtomicString for these strings. Use regular String instead. > Source/WebCore/editing/EditCommand.cpp:46 > + return "formatJustifyLeft"; Please wrap these strings in ASCIILiteral. > Source/WebCore/testing/Internals.cpp:3376 > +void Internals::executeEditingCommand(const String& commandName) const We already have this facility as testRunner.execCommand: https://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl#L87 Please use that instead. > LayoutTests/fast/events/before-input-prevent-biu-expected.txt:2 > + You should add an instruction on how to manually test this in a browser by using description() function. > LayoutTests/fast/events/before-input-prevent-biu-expected.txt:8 > +PASS firstChild.tagName is undefined. > +PASS firstChild.textContent is 'abc' > +PASS secondChild.tagName is 'I' > +PASS secondChild.textContent is 'def' > +PASS secondChild.children[0].tagName is 'U' These outputs don't tell us why these outputs are correct. > LayoutTests/fast/events/before-input-prevent-biu.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> This is an old obsolete doctype. Please use modern doctype: <!DOCTYPE html> instead. > LayoutTests/fast/events/before-input-prevent-biu.html:36 > + shouldBeUndefined("firstChild.tagName"); > + shouldBe("firstChild.textContent", "'abc'"); > + shouldBe("secondChild.tagName", "'I'"); > + shouldBe("secondChild.textContent", "'def'"); > + shouldBe("secondChild.children[0].tagName", "'U'"); This is not a great way of testing the output of editing operations, which can easily change over time as we update editing code. Use LayoutTests/resources/dump-as-markup.js and Markup.dump('foo') after each call to keyDown & execCommand. (you don't need js-test-pre.js then). > LayoutTests/fast/events/before-input-prevent-biu.html:47 > +<body onload=beginTest()> There's no need to wait for unload event. You can just execute the script right away once div is parsed. e.g. place the script element above right between div & script element below. > LayoutTests/fast/events/before-input-prevent-cut-expected.txt:6 > +Initial value: "helloworld" > +After the prevented cut: "helloworld" > +After the actual cut: "" Again, this output isn't clear as to what this test is testing and why these results are expected. Using Markup.dump would make the semantics clear because it'll show selection, DOM, etc... > LayoutTests/fast/events/before-input-prevent-cut.html:34 > + internals.executeEditingCommand("Cut"); For cut, copy, & paste, you can just invoke the regular execCommand. We enable all these commands inside DRT & WTR.

Wenson Hsieh

Comment 11 2016-10-08 11:15:20 PDT

Comment on attachment 291007 [details] Fix Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=291007&action=review >> Source/WebCore/editing/EditCommand.cpp:41 >> +AtomicString inputTypeNameForEditingAction(EditAction action) > > We shouldn't really be using AtomicString for these strings. Use regular String instead. Ah, right -- changed to use String. >> Source/WebCore/editing/EditCommand.cpp:46 >> + return "formatJustifyLeft"; > > Please wrap these strings in ASCIILiteral. Done. >> Source/WebCore/testing/Internals.cpp:3376 >> +void Internals::executeEditingCommand(const String& commandName) const > > We already have this facility as testRunner.execCommand: > https://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl#L87 > > Please use that instead. Oh, awesome! Changed to use execCommand. >> LayoutTests/fast/events/before-input-prevent-biu-expected.txt:2 >> + > > You should add an instruction on how to manually test this in a browser by using description() function. Sounds good -- done. >> LayoutTests/fast/events/before-input-prevent-biu-expected.txt:8 >> +PASS secondChild.children[0].tagName is 'U' > > These outputs don't tell us why these outputs are correct. Good point. See below (changed to use Markup.dump). >> LayoutTests/fast/events/before-input-prevent-biu.html:1 >> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> > > This is an old obsolete doctype. Please use modern doctype: <!DOCTYPE html> instead. Good catch. Fixed. >> LayoutTests/fast/events/before-input-prevent-biu.html:36 >> + shouldBe("secondChild.children[0].tagName", "'U'"); > > This is not a great way of testing the output of editing operations, which can easily change over time as we update editing code. > Use LayoutTests/resources/dump-as-markup.js and Markup.dump('foo') after each call to keyDown & execCommand. > (you don't need js-test-pre.js then). I changed these tests to use dump-as-markup.js. Thanks for the tip! >> LayoutTests/fast/events/before-input-prevent-biu.html:47 >> +<body onload=beginTest()> > > There's no need to wait for unload event. > You can just execute the script right away once div is parsed. > e.g. place the script element above right between div & script element below. Got it. Moved the script tags to the body after the element. >> LayoutTests/fast/events/before-input-prevent-cut-expected.txt:6 >> +After the actual cut: "" > > Again, this output isn't clear as to what this test is testing and why these results are expected. > Using Markup.dump would make the semantics clear because it'll show selection, DOM, etc... Changed to use Markup.dump and Markup.description. >> LayoutTests/fast/events/before-input-prevent-cut.html:34 >> + internals.executeEditingCommand("Cut"); > > For cut, copy, & paste, you can just invoke the regular execCommand. We enable all these commands inside DRT & WTR. Yep -- changed to use execCommand.

Wenson Hsieh

Comment 12 2016-10-08 11:22:23 PDT

Created attachment 291016 [details] Refactored layout tests and added a new one.

Wenson Hsieh

Comment 13 2016-10-08 11:42:08 PDT

Comment on attachment 291016 [details] Refactored layout tests and added a new one. View in context: https://bugs.webkit.org/attachment.cgi?id=291016&action=review > Source/WebCore/editing/InsertListCommand.h:47 > + bool preservesTypingStyle() const final { return true; } Actually, I should update these in a separate patch.

Wenson Hsieh

Comment 14 2016-10-09 19:39:10 PDT

Created attachment 291058 [details] Patch for landing

WebKit Commit Bot

Comment 15 2016-10-09 20:13:54 PDT

Comment on attachment 291058 [details] Patch for landing Clearing flags on attachment: 291058 Committed r206979: <http://trac.webkit.org/changeset/206979>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component UI Events

Assignee

Wenson Hsieh

Reported

2016-10-06 09:59 PDT

Modified

2016-10-09 22:34 PDT History

CC List

5 users Show

URL

Keywords InRadar

Depends on

163021

Blocks

163112

Dependancies

tree graph