Drag and drop is always moving the content even if copy is requested (by pressing the Control key when dropping).
Created attachment 301753 [details] Patch
Comment on attachment 301753 [details] Patch Attachment 301753 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3133120 New failing tests: editing/pasteboard/drag-drop-copy-content.html
Created attachment 301759 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 301753 [details] Patch Attachment 301753 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3133173 New failing tests: editing/pasteboard/drag-drop-copy-content.html
Created attachment 301760 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 301753 [details] Patch So I see two problems here: (a) The test is failing for Mac WK1. Does Mac have the equivalent of ctrl+drag? If so, the test should be marked as failing until it can be implemented for Mac. If not, I guess it should be skipped. (b) GDK supports lots of other drag actions. DEFAULT, COPY, MOVE, LINK, PRIVATE, ASK. Is this the only additional one that we want to support in WebKit? Should we file bugs to implement the others?
(In reply to comment #6) > (a) The test is failing for Mac WK1. Does Mac have the equivalent of > ctrl+drag? If so, the test should be marked as failing until it can be > implemented for Mac. If not, I guess it should be skipped. If you see the test you will see that they do have, it's pressing alt key while dropping. I'm going to fix it for the Mac WK1 (once I will find how to run the test locally on Mac using WK1). > (b) GDK supports lots of other drag actions. DEFAULT, COPY, MOVE, LINK, > PRIVATE, ASK. Is this the only additional one that we want to support in > WebKit? Should we file bugs to implement the others? I really don't know. I think the just MOVE and COPY is fine (and I think that we are advocating support for these two for the web view).
(In reply to comment #7) > (In reply to comment #6) > > (a) The test is failing for Mac WK1. Does Mac have the equivalent of > > ctrl+drag? If so, the test should be marked as failing until it can be > > implemented for Mac. If not, I guess it should be skipped. > > If you see the test you will see that they do have, it's pressing alt key > while dropping. I'm going to fix it for the Mac WK1 (once I will find how to > run the test locally on Mac using WK1). OK, it looks like WK1 doesn't support it on Mac (if tried in MiniBrowser -1), but it works on WK2 if tried through Safari. I will skip the test on WK1 then.
Created attachment 301906 [details] Patch Marked the layout test as failing in the mac WK1 and created a bug 168503 for it. Previously I wrote that it does not work on WK1, but that was because I was connected through VNC to the mac machine, and the alt key doesn't work there.
Comment on attachment 301906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301906&action=review LGTM. I'd like Carlos to review it too. > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:32 > + if (isGtk() || isEfl()) EFL is gone now, so remove this.
Comment on attachment 301906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301906&action=review > LayoutTests/ChangeLog:10 > + Mark the editing/pasteboard/drag-drop-copy-content.html as failing > + as WTR doesn't know how to perform drag and drop in WK2. Also the > + test does not pass on the mac WK1, created a bug for it. So, we are adding a test that doesn't pass in any platform? > LayoutTests/editing/pasteboard/drag-drop-copy-content-expected.txt:3 > +FAIL: input Shouldn't this be PASS? According to the test when it fails it says FAIL: expected value="Text", actual value=, no? > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:7 > +function debug(msg) { This is the actual test result, not a debug message no? I would call this log instead of debug if that is the case. > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:14 > +function editingTest() { I would call this runTest() > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:42 > + testRunner.notifyDone(); I'm not sure we need to use wait/notify in a test like this. > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:47 > +<p>This tests the generic drag and drop where the copy of the content is > +requested (pressing Control on WebKitGTK+ or Alt on Mac when dropping the > +content) instead of the move.</p> Since DND can't be tested by WTR in WK2, ity would be useful to have a manual way to test this, explained here. Other test do that and it's useful to check that the test works using MiniBrowser.
(In reply to comment #11) > > LayoutTests/ChangeLog:10 > > + Mark the editing/pasteboard/drag-drop-copy-content.html as failing > > + as WTR doesn't know how to perform drag and drop in WK2. Also the > > + test does not pass on the mac WK1, created a bug for it. > > So, we are adding a test that doesn't pass in any platform? Yes, but it will start to pass once the bugs are fixed. > > LayoutTests/editing/pasteboard/drag-drop-copy-content-expected.txt:3 > > +FAIL: input > > Shouldn't this be PASS? According to the test when it fails it says FAIL: > expected value="Text", actual value=, no? It definitely needs to be PASS and I had it like that, but it probably got replaced by run-webkit-tests. :/ > > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:7 > > +function debug(msg) { > > This is the actual test result, not a debug message no? I would call this > log instead of debug if that is the case. > > > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:14 > > +function editingTest() { > > I would call this runTest() > > > LayoutTests/editing/pasteboard/drag-drop-copy-content.html:42 > > + testRunner.notifyDone(); > > I'm not sure we need to use wait/notify in a test like this. The test is based on editing/pasteboard/drag-drop-input-textarea.html and it does include these as well, but I will rework it.
Created attachment 302123 [details] Patch
Comment on attachment 302123 [details] Patch Clearing flags on attachment: 302123 Committed r212881: <http://trac.webkit.org/changeset/212881>
All reviewed patches have been landed. Closing bug.
There was a followup fix to this: http://trac.webkit.org/projects/webkit/changeset/213638