repro: (1) Keeping the ctrl key down, and drag a link (2) Drag it over a "div" element (3) On the onDragOver event of the "div" element, testing the event.ctrlKey property, its always "false". It should be "true" ------- HTML source repro this: ------- <html> <script language="javascript"> window.onload = function() { var oDrop = document.getElementById("divDrop"); oDrop.addEventListener("dragover", onDragOverDiv, false); oDrop.addEventListener("dragenter", onDragOverDiv, false); } function onDragOverDiv(e) { e = e || window.event; e.dataTransfer.dropEffect = "copy"; shwTxt("e.ctrlKey == : " + e.ctrlKey); } function shwTxt(s) { document.getElementById("divOutput").innerHTML += "<br>" + s; } </script> <body> <div id="divDrop">drop on me</div> <a href="www.google.com" id="divDrag">Drag me</a> <div id="divOutput" style="top:200px"></div> </body> </html>
Created attachment 18814 [details] test case Same test, as an attachment.
Confirmed with r29869.
Should be noted that this isn't just true for the control key. It's all keyboard information (shift, meta, alt, etc.). Section 7.9.3 of HTML5 states: "the mouse and key attributes [must be] set according to the state of the input devices as they would be for user interaction events"
Created attachment 50662 [details] Implement returning the correct state of the modifier keys being pressed or not during a drag for Mac, Chromium, Haiku, and Windows platforms. Tested on Mac OS X 10.6 As noted in the ChangeLog, did not add in tests here because of a bug with DumpRenderTree: I have written a test for the Mac platform that is not included in this patch and depends on https://bugs.webkit.org/show_bug.cgi?id=36085 (DumpRenderTree eventSender.mouseDown should change NSApp currentEvent return value) being fixed. Should I include it in this fix and just add it to the skip list until that bug is fixed? Also, should I split off this bug into multiple smaller bugs, one per each platform which I did not provide an implementation? (Qt, GTK, EFL, WX)
Comment on attachment 50662 [details] Implement returning the correct state of the modifier keys being pressed or not during a drag for Mac, Chromium, Haiku, and Windows platforms. Will post a patch soon that usesPlatformKeyboardEvent::getCurrentModifierState() from http://trac.webkit.org/changeset/55909 instead.
Created attachment 50665 [details] Use the new PlatformKeyboardEvent::getCurrentModifierState() to add the information about which modifier keys are pressed to the drag event As in the last patch, I did not add in tests here (hence the No new tests (OOPS!) in the ChangeLog) because of a bug with DumpRenderTree, similar to the bug https://bugs.webkit.org/show_bug.cgi?id=36085 . At least on the Mac, the mechanism used to implement getCurrentModifierState (GetCurrentKeyModifiers()) does not appear to get modified by the eventSender.mouseDown method. Should I include that failing test in this fix and just add it to the skip list for now? Any ideas on what might be used to make GetCurrentKeyModifiers (which appears to be a Carbon method) use the information from the eventSender.mouseDown method? The other option would be a manual test, but I talked earlier to Krit, and he strongly suggested against that (for the good reason that manual tests are less likely to get run).
Comment on attachment 50665 [details] Use the new PlatformKeyboardEvent::getCurrentModifierState() to add the information about which modifier keys are pressed to the drag event Resolving bug 36085 will not affect the testability of this change, because getCurrentModifierState does not use -currentEvent on Mac OS X. I think it cannot use -currentEvent because changes to modifier key state don’t result in an event, but I could be wrong.
(In reply to comment #7) > (From update of attachment 50665 [details]) > Resolving bug 36085 will not affect the testability of this change Right, resolving that bug won't do anything in this case. It would have helped with my previous patch. However, I am still fairly sure that there is an issue with eventSender.mouseDown that is causing my layout test to fail while my manual testing works. Should I just leave this with no tests? > , because > getCurrentModifierState does not use -currentEvent on Mac OS X. I think it > cannot use -currentEvent because changes to modifier key state don’t result in > an event, but I could be wrong. Do you know of any way to essentially trick getCurrentModifierState into believing that there is a modifier key being held down? As it currently stands, eventSender.mouseDown just sends the new event with the modifier key state set according to the arguments to eventSender.mouseDown to the NSView. If there is a way to get that information to getCurrentModifierState, then I am pretty sure my layout test would work.
As an aside, it's rather suspicious that PlatformKeyboardEvent::getCurrentModifierState() uses GetCurrentKeyModifiers(), and not GetCurrentEventKeyModifiers(). But I don't think that even the latter will be affected by anything done at AppKit level.
Comment on attachment 50665 [details] Use the new PlatformKeyboardEvent::getCurrentModifierState() to add the information about which modifier keys are pressed to the drag event Please remove the Oops before landing.
(In reply to comment #9) > As an aside, it's rather suspicious that > PlatformKeyboardEvent::getCurrentModifierState() uses GetCurrentKeyModifiers(), > and not GetCurrentEventKeyModifiers(). Why? It is used (in EventHandler.cpp) to fabricate a fake event with the current state of the modifier keys, not their last known state.
That's I didn't say it was incorrect, just suspicious. Why do we want to use modifier state that isn't event-synchronized?
Comment on attachment 50665 [details] Use the new PlatformKeyboardEvent::getCurrentModifierState() to add the information about which modifier keys are pressed to the drag event Committed in r55974: http://trac.webkit.org/changeset/55974
Looking at this change though, I'd say that using non-synchronized modifier state is likely incorrect! This difference can become noticeable on a busy machine. Jessie, what do you think?
(In reply to comment #14) > Looking at this change though, I'd say that using non-synchronized modifier > state is likely incorrect! This difference can become noticeable on a busy > machine. > > Jessie, what do you think? I could see how on a busy / slow enough machine if we know that GetCurrentEventKeyModifiers remains in sync with the slower execution of this DragController code, then it might be correct to use GetCurrentEventKeyModifiers instead of GetCurrentKeyModifiers (which might report the immediate state instead of the one in sync with the slowed down DragController code). However, do we know that the GetCurrentEventKeyModifiers would necessarily remain in sync with slow execution of the DragController code? Or is this an unrealistic worry? If we don't, then I think either solution would work.
If this code runs as part of handling an event, shouldn't it just pass the event along? I'm confused.
(In reply to comment #16) > If this code runs as part of handling an event, shouldn't it just pass the > event along? I'm confused. It doesn't run directly as part of handling the actual dragging event, at least on the Mac. Once DragController is told to start the drag as a result of the EventHandler figuring out that the mouse move event is actually a drag, it hands it off to doSystemDrag. On the Mac, this means that it goes into NSCoreDragManager. When NSCoreDragTrackingProc tells the WebView about the dragging, it doesn't give it an event, it gives it an NSDraggingInfo, in which there is a lot of the information that would be in an event but not the information about the state of the keys that are pressed. As I understand it, that is why there is no event to pass along and DragController has to create a PlatformMouseEvent to work with. We could store the original event when the drag got started and use the information from that, but that might become incorrect if the user let go of some keys in the process of doing the drag. I don't know what event would be the "CurrentEvent" used by GetCurrentEventKeyModifiers, but I was going along the assumption that it would be more recent than the original one that caused DragController to call doSystemDrag in the first place but might be in sync with the DragController code executing when I was responding to Alexey's question. That probably was not an assumption I should have made.