The image in this test is not copied properly. This can be observed by looking at the pixel results.
Created attachment 81157 [details] Patch This issue was that images were not written to the markup and URL portion of the clipboard. I've corrected this in my attached patch.
Comment on attachment 81157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81157&action=review > Source/WebCore/platform/gtk/PasteboardGtk.cpp:115 > + // FIXME: Later this code should be shared with Chromium somehow. Chances are all platforms want it. Which code? Can we abstract said code into a helper method (even if we don't share it now)? > Source/WebCore/platform/gtk/PasteboardGtk.cpp:130 > + if (!url.isEmpty()) { This whole block feels like a helper method to me. :)
Comment on attachment 81157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81157&action=review >> Source/WebCore/platform/gtk/PasteboardGtk.cpp:130 >> + if (!url.isEmpty()) { > > This whole block feels like a helper method to me. :) Indeed, it seems like we can add a function to markup.h/cpp.
Comment on attachment 81157 [details] Patch r- due to style issues.
Created attachment 89416 [details] Updated patch
(In reply to comment #2) > (From update of attachment 81157 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=81157&action=review Thank you both for your comments! > > Source/WebCore/platform/gtk/PasteboardGtk.cpp:115 > > + // FIXME: Later this code should be shared with Chromium somehow. Chances are all platforms want it. > Which code? Can we abstract said code into a helper method (even if we don't share it now)? I've abstracted this code into a helper. > > Source/WebCore/platform/gtk/PasteboardGtk.cpp:130 > > + if (!url.isEmpty()) { > eric: This whole block feels like a helper method to me. :) > rniwa: Indeed, it seems like we can add a function to markup.h/cpp. I've abstracted this into a helper based on a version of this code in the chromium platform directory. I've also replaced another implementation of it in the win platform directory.
Attachment 89416 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8401399
Created attachment 89439 [details] Fix build for Chromium and Windows
Attachment 89416 [details] did not build on win: Build output: http://queues.webkit.org/results/8401402
Attachment 89439 [details] did not build on win: Build output: http://queues.webkit.org/results/8403349
Created attachment 89587 [details] Try to fix the Windows build
Attachment 89587 [details] did not build on win: Build output: http://queues.webkit.org/results/8404661
Created attachment 89593 [details] One more shot at fixing the Windows build
Comment on attachment 89593 [details] One more shot at fixing the Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=89593&action=review This looks sane to me. > Source/WebCore/ChangeLog:26 > + * platform/win/ClipboardWin.cpp: Remove the imageToMarkup helper and the and the -> and use the
Committed r85064: <http://trac.webkit.org/changeset/85064>
Some time ago I noticed Chrome has some weird behavior regarding drag and drop due to imageToMarkup(): https://bugs.webkit.org/show_bug.cgi?id=58043 Unfortunately I had no time to pursue at the time and now I see imageToMarkup has been generalized :) Is there a reason why you spread imageToMarkup() instead of using the existing createMarkup()?