Bug 162954 - Introduce InputEvent bindings in preparation for the input events spec
Summary: Introduce InputEvent bindings in preparation for the input events spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on: 162997
Blocks: 163112
  Show dependency treegraph
 
Reported: 2016-10-04 22:43 PDT by Wenson Hsieh
Modified: 2016-10-07 07:31 PDT (History)
6 users (show)

See Also:


Attachments
First pass (38.31 KB, patch)
2016-10-05 08:40 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Added InputEvent to CMakeLists. (39.03 KB, patch)
2016-10-05 08:47 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (969.24 KB, application/zip)
2016-10-05 09:58 PDT, Build Bot
no flags Details
Added InputEvent to constructor test expectations. (46.04 KB, patch)
2016-10-05 10:00 PDT, Wenson Hsieh
rniwa: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 (14.98 MB, application/zip)
2016-10-05 11:30 PDT, Build Bot
no flags Details
Patch for landing (50.01 KB, patch)
2016-10-05 20:33 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>