RESOLVED FIXED 47244
[GTK] editing/pasteboard/dataTransfer-setData-getData.html fails on GTK+
https://bugs.webkit.org/show_bug.cgi?id=47244
Summary [GTK] editing/pasteboard/dataTransfer-setData-getData.html fails on GTK+
Martin Robinson
Reported 2010-10-05 20:44:20 PDT
The issue here seems to be that setData('URL') and setData('text/uri-list') do not preserve the original string. Patch forthcoming.
Attachments
Patch (28.80 KB, patch)
2010-10-05 21:44 PDT, James Simonsen
no flags
Patch for this issue (20.73 KB, patch)
2010-10-06 11:20 PDT, Martin Robinson
no flags
Patch with Tony's suggestions (20.77 KB, patch)
2010-10-06 12:15 PDT, Martin Robinson
no flags
Martin Robinson
Comment 1 2010-10-05 20:54:05 PDT
James Simonsen
Comment 2 2010-10-05 21:44:53 PDT
James Simonsen
Comment 3 2010-10-05 21:46:20 PDT
Sorry, my patch went to the wrong bug. Is there any way to delete that?
Martin Robinson
Comment 4 2010-10-06 11:20:31 PDT
Created attachment 69966 [details] Patch for this issue
Tony Chang
Comment 5 2010-10-06 11:43:30 PDT
Comment on attachment 69966 [details] Patch for this issue View in context: https://bugs.webkit.org/attachment.cgi?id=69966&action=review (not changing the review flag because I don't feel qualified to r+ the gtk+ specific code) > WebCore/platform/gtk/DataObjectGtk.cpp:66 > + // This code is originally from: platform/chromium/ChromiumDataObject.cpp. Can you add a FIXME that says we should try to refactor this code for sharing across platforms? > WebCore/platform/gtk/DataObjectGtk.cpp:104 > void DataObjectGtk::setURL(const KURL& url, const String& label) > { > + m_url = url; This seems redundant since setURIList should do this for you. > WebCore/platform/gtk/PasteboardHelper.cpp:141 > + CString uriList = dataObject->uriList().utf8(); > + gtk_selection_data_set(selectionData, uriListAtom, 8, > + reinterpret_cast<const guchar*>(uriList.data()), strlen(uriList.data()) + 1); Would urList.length() + 1 be better than calling strlen?
Martin Robinson
Comment 6 2010-10-06 12:15:25 PDT
Created attachment 69975 [details] Patch with Tony's suggestions
Martin Robinson
Comment 7 2010-10-06 12:17:25 PDT
(In reply to comment #5) Thanks for the comments. > > WebCore/platform/gtk/DataObjectGtk.cpp:66 > > + // This code is originally from: platform/chromium/ChromiumDataObject.cpp. > Can you add a FIXME that says we should try to refactor this code for sharing across platforms? Done. > > WebCore/platform/gtk/DataObjectGtk.cpp:104 > > void DataObjectGtk::setURL(const KURL& url, const String& label) > > { > > + m_url = url; > This seems redundant since setURIList should do this for you. Instead of using setURIList here, I've switched it to simply do m_uriList = url;, to avoid any unintended side-effects (like settings files on the DataObject). > > WebCore/platform/gtk/PasteboardHelper.cpp:141 > > + CString uriList = dataObject->uriList().utf8(); > > + gtk_selection_data_set(selectionData, uriListAtom, 8, > > + reinterpret_cast<const guchar*>(uriList.data()), strlen(uriList.data()) + 1); > Would urList.length() + 1 be better than calling strlen? It definitely is! Fixed.
Tony Chang
Comment 8 2010-10-11 11:32:10 PDT
Comment on attachment 69975 [details] Patch with Tony's suggestions I still think it would be nice if someone more familiar with the GTK+ port would review.
Martin Robinson
Comment 9 2010-10-11 15:13:10 PDT
Comment on attachment 69975 [details] Patch with Tony's suggestions Clearing flags on attachment: 69975 Committed r69530: <http://trac.webkit.org/changeset/69530>
Martin Robinson
Comment 10 2010-10-11 15:15:28 PDT
Note You need to log in before you can comment on or make changes to this bug.