stdweb icon indicating copy to clipboard operation
stdweb copied to clipboard

Drag events are broken in Webkit

Open malleusinferni opened this issue 6 years ago • 6 comments

The HTML drag-and-drop API specifies the class DragEvent, and several subclasses thereof. Presently, Webkit does not implement any of these classes or define any of their constructors; their names are unbound. Instead, drag events are dispatched using instances of MouseEvent (from which DragEvent inherits) with the necessary properties added after instantiation.

None of this matters to typical JavaScript code, since it's written in a duck-typed style with no awareness of inheritance hierarchies. However, several stdweb features explicitly query the classes of JavaScript objects when deciding how to bridge them to Rust. When a script listens for a DragEvent instance, it will therefore evaluate the DragEvent variable, which in Webkit is undefined. In my tests, triggering the dispatch of such an event produced a ReferenceError that crashed the page.

The actual problem here is with Webkit, but I don't know that it makes sense to wait for them to fix it. There's an outstanding patch from 2015 that intended to address this. It was apparently a work in progress and never reached review.

The problem does not affect Chrome, which seems to have patched it after the Blink fork.

malleusinferni avatar Dec 22 '18 22:12 malleusinferni

Hmmm... this is an interesting issue. I'm somewhat at a loss on how we could fix this. On one hand I don't really want to affect the functionality on the standards conformant browsers, on the other hand Safari has a significant market share.

Perhaps we could include a polyfill which would simply define DragEvent as MouseEvent if the DragEvent is undefined?

koute avatar Dec 23 '18 22:12 koute

One approach I'd like to try is to look at event.type. Webkit correctly sets this property, e.g. a DragOver event will report event.type == 'dragover', even though event.constructor.name == 'MouseEvent' and the name DragOver is unbound. It also populates the dataTransfer property on drag events, since it's needed by client code. I suspect stdweb should still be able to expose IDragEvent on these objects, as long as it can somehow avoid the instanceof test. Right now, unfortunately, I don't know my way around the codebase well enough to implement this change.

malleusinferni avatar Dec 24 '18 19:12 malleusinferni

One approach I'd like to try is to look at event.type.

We already do this. (:

However, that isn't really sufficient as 1) it would also accept plain objects which are not events but only coincidentally set the type, and 2) we also have the DragRelatedEvent type which is supposed to be a concrete object for every event which implements IDragEvent, so if we can't check for instanceof DragEvent then what are we supposed to check to detect any drag event object?

Again, wouldn't the easiest and the simplest fix would be simply a polyfill for DragEvent? Something like this maybe:

if( typeof window !== "undefined" && typeof DragEvent === "undefined" ) {
    window.DragEvent = MouseEvent;
}

koute avatar Dec 25 '18 13:12 koute

Oh, I completely misunderstood what you meant by "define DragEvent as MouseEvent." I've only performed limited tests but this snippet seems to fix almost everything. I'll take a closer look in a few days. Thanks!

malleusinferni avatar Jan 04 '19 16:01 malleusinferni

@malleusinferni If that will end up working I'm fine with merging it into stdweb considering it's pretty simple and non-invasive.

koute avatar Jan 04 '19 20:01 koute

Yep, I'm good with this. I thought I saw some other weirdness, but I can't reproduce it now.

malleusinferni avatar Jan 21 '19 00:01 malleusinferni