Bug 162954

Summary: Introduce InputEvent bindings in preparation for the input events spec
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: UI EventsAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, buildbot, commit-queue, enrica, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 162997    
Bug Blocks: 163112    
Attachments:
Description Flags
First pass
none
Added InputEvent to CMakeLists.
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Added InputEvent to constructor test expectations.
rniwa: review+, buildbot: commit-queue-
Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2
none
Patch for landing none

Description Wenson Hsieh 2016-10-04 22:43:09 PDT
Introduce InputEvent bindings in preparation for the input events spec
Comment 1 Wenson Hsieh 2016-10-05 08:40:55 PDT
Created attachment 290708 [details]
First pass
Comment 2 Wenson Hsieh 2016-10-05 08:47:31 PDT
Created attachment 290709 [details]
Added InputEvent to CMakeLists.
Comment 3 Wenson Hsieh 2016-10-05 08:53:31 PDT
Comment on attachment 290709 [details]
Added InputEvent to CMakeLists.

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

> Source/WebCore/dom/InputEvent.h:56
> +    void initInputEvent(const String& inputType);

I realized I don't actually need this method -- removed.
Comment 4 Build Bot 2016-10-05 09:58:16 PDT
Comment on attachment 290709 [details]
Added InputEvent to CMakeLists.

Attachment 290709 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2224980

New failing tests:
js/dom/global-constructors-attributes.html
Comment 5 Build Bot 2016-10-05 09:58:18 PDT
Created attachment 290718 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 6 Wenson Hsieh 2016-10-05 10:00:27 PDT
Created attachment 290719 [details]
Added InputEvent to constructor test expectations.
Comment 7 Sam Weinig 2016-10-05 11:01:39 PDT
Comment on attachment 290719 [details]
Added InputEvent to constructor test expectations.

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

> Source/WebCore/testing/InternalSettings.h:175
> +    bool inputEventsEnabled(ExceptionCode&);
> +    void setInputEventsEnabled(bool, ExceptionCode&);

Are these necessary? I would assume they would be generated automatically in InternalSettingsGenerated since you put this in Settings.in
Comment 8 Wenson Hsieh 2016-10-05 11:09:03 PDT
Comment on attachment 290719 [details]
Added InputEvent to constructor test expectations.

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

>> Source/WebCore/testing/InternalSettings.h:175
>> +    void setInputEventsEnabled(bool, ExceptionCode&);
> 
> Are these necessary? I would assume they would be generated automatically in InternalSettingsGenerated since you put this in Settings.in

I believe these definitions are still necessary, since this is InternalSettings. It's not needed in Settings.h, since that uses Settings.in to generate the relevant getters/setters in SettingsMacros.h and insert them via the SETTINGS_GETTERS_AND_SETTERS macro.
Comment 9 Wenson Hsieh 2016-10-05 11:14:43 PDT
(In reply to comment #8)
> Comment on attachment 290719 [details]
> Added InputEvent to constructor test expectations.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290719&action=review
> 
> >> Source/WebCore/testing/InternalSettings.h:175
> >> +    void setInputEventsEnabled(bool, ExceptionCode&);
> > 
> > Are these necessary? I would assume they would be generated automatically in InternalSettingsGenerated since you put this in Settings.in
> 
> I believe these definitions are still necessary, since this is
> InternalSettings. It's not needed in Settings.h, since that uses Settings.in
> to generate the relevant getters/setters in SettingsMacros.h and insert them
> via the SETTINGS_GETTERS_AND_SETTERS macro.

Never mind, I see what you mean. The version of the getter/setter I defined in InternalSettings is one with exceptions, which isn’t generated automatically in derived sources. It looks like it's being invoked by JS bindings, since I specified in the IDL that the setter/getter may throw exceptions. Should I remove these exception-throwing versions of the getter/setter?
Comment 10 Build Bot 2016-10-05 11:30:21 PDT
Comment on attachment 290719 [details]
Added InputEvent to constructor test expectations.

Attachment 290719 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2225350

New failing tests:
fast/events/input-events-fired-when-typing.html
Comment 11 Build Bot 2016-10-05 11:30:24 PDT
Created attachment 290733 [details]
Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 12 Ryosuke Niwa 2016-10-05 16:04:59 PDT
Comment on attachment 290719 [details]
Added InputEvent to constructor test expectations.

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

r=me assuming you address Sam's comment and fix or skip the test on iOS.

> LayoutTests/fast/events/input-events-fired-when-typing.html:27
> +            eventSender.keyDown("a", []);

You probably need to skip this test on iOS due to eventSender not working on iOS.

> LayoutTests/fast/events/input-events-fired-when-typing.html:38
> +            shouldBe("event.target.id", "expectedTargetID");

Please also check the values of bubbles, cancelable, and composed.
Comment 13 Wenson Hsieh 2016-10-05 20:33:45 PDT
Created attachment 290775 [details]
Patch for landing
Comment 14 WebKit Commit Bot 2016-10-05 21:08:05 PDT
Comment on attachment 290775 [details]
Patch for landing

Clearing flags on attachment: 290775

Committed r206843: <http://trac.webkit.org/changeset/206843>
Comment 15 Wenson Hsieh 2016-10-06 13:50:04 PDT
<rdar://problem/28658043>