Bug 205758 – Add support for DragEvent

• Component: HTML Editing
• Status: RESOLVED FIXED

[Chris Dumez]

Add support for DragEvent:
- https://html.spec.whatwg.org/multipage/dnd.html#the-dragevent-interface

Both Blink and Gecko support it. In WebKit, no DragEvent and we added the dataTransfer attribute to MouseEvent instead.

[Chris Dumez]

Created attachment 386726 [details]
WIP Patch

[Chris Dumez]

Created attachment 386728 [details]
WIP Patch

[Chris Dumez]

Created attachment 386731 [details]
WIP Patch

[Chris Dumez]

Created attachment 386733 [details]
WIP Patch

[Chris Dumez]

Created attachment 386736 [details]
WIP Patch

[Chris Dumez]

Created attachment 386739 [details]
Patch

[Chris Dumez]

Created attachment 386745 [details]
Patch

[Darin Adler]

Comment on attachment 386745 [details]
Patch

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

> Source/WebCore/dom/DragEvent.h:47
> +    static Ref<DragEvent> create(const AtomString& eventType, DragEventInit&& init)
> +    {
> +        return adoptRef(*new DragEvent(eventType, WTFMove(init)));
> +    }

There's no good reason for this function body to be in the header; inlining this at the call site isn’t really valuable since it’s going to call a constructor, which is not inlined. Instead we can let the constructor get inlined in the create function. And so the create function can be in the .cpp file. Nice to have a little less code in the header, I think.

> Source/WebCore/dom/DragEvent.h:52
> +    static Ref<DragEvent> createForBindings()
> +    {
> +        return adoptRef(*new DragEvent);
> +    }

Ditto.

> Source/WebCore/dom/DragEvent.idl:33
> +    DataTransfer? dataTransfer = null;

Is the "= null" really required?

> Source/WebCore/page/EventHandler.cpp:2255
> +    // FIXME: Use DragEvent::create which takes PlatformMouseEvent.

Is this FIXME valuable? Does the suggestec change need to be made? Would it improve things?

> LayoutTests/fast/events/constructors/mouse-event-constructor.html:30
> +shouldBe("new MouseEvent('eventType').dataTransfer", "undefined");

More than just "should be undefined". The property shouldn’t even be present! Might be better to check that. Something ike this:

    shouldBeFalse("dataTransfer in (new MouseEvent('eventType'))")

[Chris Dumez]

Created attachment 386749 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 386749 [details]
Patch

Clearing flags on attachment: 386749

Committed r254029: <https://trac.webkit.org/changeset/254029>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58314184>

[Simon Fraser (smfr)]

Build fix in bug 205765.