Summary: | [GTK] editing/pasteboard/copy-standalone-image.html fails | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||||||||||
Component: | WebKitGTK | Assignee: | 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
Martin Robinson
2011-02-02 17:00:19 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.
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()? |