Bug 163025

Summary:

Support InputEvent.inputType for the new InputEvent spec

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

UI Events

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, gyuyoung.kim, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

163021

Bug Blocks:

163112

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews102 for mac-yosemite	none
Archive of layout-test-results from ews116 for mac-yosemite	none
Fix DRT failures.	none
Rebased on ToT	none
Fix Windows build	none
Refactored layout tests and added a new one.	darin: review+
Patch for landing	none

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.