Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for more InputEvent initialization paramaters, such as dataTransfer and targetRanges #19346

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MoeBazziGIT
Copy link
Contributor

@MoeBazziGIT MoeBazziGIT commented Oct 20, 2023

…such as dataTransfer and targetRanges

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

Reviewed by NOBODY (OOPS!).

* Source/WebCore/dom/InputEvent.cpp:
(WebCore::InputEvent::InputEvent):
* Source/WebCore/dom/InputEvent.h:
* Source/WebCore/dom/InputEvent.idl:
@MoeBazziGIT MoeBazziGIT changed the title This adds support for more initialization paramaters for InputEvent, such as dataTransfer and targetRanges Add support for more initialization paramaters for InputEvent, such as dataTransfer and targetRanges Oct 20, 2023
@MoeBazziGIT MoeBazziGIT changed the title Add support for more initialization paramaters for InputEvent, such as dataTransfer and targetRanges Add support for more InputEvent initialization paramaters, such as dataTransfer and targetRanges Oct 20, 2023
Copy link
Member

@whsieh whsieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch! A couple of comments:

  1. Move the summary describing the change in the commit message, below the bug number and change the title to be consistent with the bugzilla bug. i.e.,:
Support InputEventInit.{inputType, dataTransfer, isComposing, targetRanges}
https://bugs.webkit.org/show_bug.cgi?id=170416

Reviewed by NOBODY (OOPS!).

This adds support for more initialization parameters for InputEvent, such as dataTransfer and targetRanges.

* Source/WebCore/dom/InputEvent.cpp:
(WebCore::InputEvent::InputEvent):
* Source/WebCore/dom/InputEvent.h:
* Source/WebCore/dom/InputEvent.idl:
  1. We should add a new test (or modify existing tests) to reflect this change. The existing WPT imported/w3c/web-platform-tests/uievents/constructors/inputevent-constructor.html seems pertinent.

It seems like the spec doesn't include these two arguments, but testing in Chrome and Firefox, I can see that they both allow you to specify targetRanges and dataTransfer in InputEventInit, so this change would align with Firefox/Chrome. It seems that the spec just needs to be updated.

Comment on lines +45 to +46
Vector<RefPtr<StaticRange>> targetRanges;
RefPtr<DataTransfer> dataTransfer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit - can we keep these in an order that's consistent with the IDL? (i.e. dataTransfer and then targetRanges)

@johanneswilm
Copy link

It seems like the spec doesn't include these two arguments

@whsieh I've added it to the next Web Editing Workforce call. However, I am thinking maybe there is only some confusion because half the IDL is in the uievents spec and the other half in the input events spec. The relevant part is in the input events spec: https://w3c.github.io/input-events/#interface-InputEvent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants