Bug 53645

Summary: [GTK] editing/pasteboard/copy-standalone-image.html fails
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, dglazkov, webkit.review.bot
Priority: P3 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Updated patch
none
Fix build for Chromium and Windows
none
Try to fix the Windows build
none
One more shot at fixing the Windows build none

Martin Robinson
Reported 2011-02-02 17:00:19 PST
The image in this test is not copied properly. This can be observed by looking at the pixel results.
Attachments
Patch (47.28 KB, patch)
2011-02-03 17:38 PST, Martin Robinson
no flags
Updated patch (52.77 KB, patch)
2011-04-13 11:44 PDT, Martin Robinson
no flags
Fix build for Chromium and Windows (53.58 KB, patch)
2011-04-13 12:58 PDT, Martin Robinson
no flags
Try to fix the Windows build (53.57 KB, patch)
2011-04-14 08:42 PDT, Martin Robinson
no flags
One more shot at fixing the Windows build (53.54 KB, patch)
2011-04-14 09:35 PDT, Martin Robinson
no flags
Martin Robinson
Comment 1 2011-02-03 17:38:52 PST
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.
Eric Seidel (no email)
Comment 2 2011-04-06 10:26:10 PDT
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. :)
Ryosuke Niwa
Comment 3 2011-04-11 15:50:38 PDT
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.
Ryosuke Niwa
Comment 4 2011-04-11 15:50:57 PDT
Comment on attachment 81157 [details] Patch r- due to style issues.
Martin Robinson
Comment 5 2011-04-13 11:44:38 PDT
Created attachment 89416 [details] Updated patch
Martin Robinson
Comment 6 2011-04-13 11:47:36 PDT
(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.
WebKit Review Bot
Comment 7 2011-04-13 12:51:52 PDT
Martin Robinson
Comment 8 2011-04-13 12:58:26 PDT
Created attachment 89439 [details] Fix build for Chromium and Windows
Build Bot
Comment 9 2011-04-13 12:58:33 PDT
Build Bot
Comment 10 2011-04-13 14:17:22 PDT
Martin Robinson
Comment 11 2011-04-14 08:42:28 PDT
Created attachment 89587 [details] Try to fix the Windows build
Build Bot
Comment 12 2011-04-14 09:18:28 PDT
Martin Robinson
Comment 13 2011-04-14 09:35:18 PDT
Created attachment 89593 [details] One more shot at fixing the Windows build
Xan Lopez
Comment 14 2011-04-26 15:38:30 PDT
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
Martin Robinson
Comment 15 2011-04-27 11:16:14 PDT
Benjamin Poulain
Comment 16 2011-07-29 04:33:42 PDT
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()?
Note You need to log in before you can comment on or make changes to this bug.