REGRESSION (r107435) URLs copied from WebKit apps aren't in the right pasteboard format In writeURLForTypes in PasteboardMac.mm Before 107435: if ([types containsObject:NSURLPboardType]) [cocoaURL writeToPasteboard:pasteboard]; After 107435: if (types.contains(String(NSURLPboardType))) platformStrategies()->pasteboardStrategy()->setStringForType([cocoaURL absoluteString], NSURLPboardType, pasteboardName); URLs have to paste themselves to the pasteboard to get the right plist format the system expects. Patch forthcoming. In radar as <rdar://problem/10848575>
Created attachment 132669 [details] Patch with API test
Comment on attachment 132669 [details] Patch with API test View in context: https://bugs.webkit.org/attachment.cgi?id=132669&action=review The change looks good to me but I'll let mac port folks take a look. > Source/WebCore/ChangeLog:4 > + REGRESSION (r107435) URLs copied from WebKit apps aren't in the right pasteboard format Summary should appear before the bug urls. Also, people tend to put rdar and bugzilla urls on separate lines.
Comment on attachment 132669 [details] Patch with API test The patch is correct, but I think you could simplify it. There is no need to add a new message for webkit2, you can use the same message used to set the string type and implement a different behavior based on the pasteboard type. This way you don't need to add a new message and you need no changes to platform strategies. The only changes required are in WebContextMac.mm in setPasteboardString.
(In reply to comment #4) > (From update of attachment 132669 [details]) > The patch is correct, but I think you could simplify it. There is no need to add a new message for webkit2, you can use the same message used to set the string type and implement a different behavior based on the pasteboard type. > This way you don't need to add a new message and you need no changes to platform strategies. The only changes required are in WebContextMac.mm in setPasteboardString. I can see that being reasonable. It will - unfortunately - make this mechanism asymmetrical as the "pasting app" fix in http://trac.webkit.org/changeset/111080 added a new message. I really wish we had the ability to message KURLs but adding that is outside the scope of this patch. I'll do what you suggest.
(In reply to comment #3) > (From update of attachment 132669 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132669&action=review > > The change looks good to me but I'll let mac port folks take a look. > > > Source/WebCore/ChangeLog:4 > > + REGRESSION (r107435) URLs copied from WebKit apps aren't in the right pasteboard format > > Summary should appear before the bug urls. > Also, people tend to put rdar and bugzilla urls on separate lines. I've done my ChangeLogs like this for years in hundreds of patches. It's also not remotely difficult to find many other contributors who've done the same. I actually don't believe we have style guidelines along these lines, and a cursory glance at past ChangeLogs show there is no standard in practice. Is there some standard we are now trying to enforce and I've missed where it was discussed/published?
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 132669 [details] [details]) > > The patch is correct, but I think you could simplify it. There is no need to add a new message for webkit2, you can use the same message used to set the string type and implement a different behavior based on the pasteboard type. > > This way you don't need to add a new message and you need no changes to platform strategies. The only changes required are in WebContextMac.mm in setPasteboardString. > > I can see that being reasonable. It will - unfortunately - make this mechanism asymmetrical as the "pasting app" fix in http://trac.webkit.org/changeset/111080 added a new message. > > I really wish we had the ability to message KURLs but adding that is outside the scope of this patch. > > I'll do what you suggest. I had not seen the other patch. I wish I had a chance to chime in.
(In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 132669 [details] [details] [details]) > > > The patch is correct, but I think you could simplify it. There is no need to add a new message for webkit2, you can use the same message used to set the string type and implement a different behavior based on the pasteboard type. > > > This way you don't need to add a new message and you need no changes to platform strategies. The only changes required are in WebContextMac.mm in setPasteboardString. > > > > I can see that being reasonable. It will - unfortunately - make this mechanism asymmetrical as the "pasting app" fix in http://trac.webkit.org/changeset/111080 added a new message. > > > > I really wish we had the ability to message KURLs but adding that is outside the scope of this patch. > > > > I'll do what you suggest. > > I had not seen the other patch. I wish I had a chance to chime in. You were singing in New York. ;)
Comment on attachment 132669 [details] Patch with API test Attachment 132669 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11989371
Created attachment 132690 [details] Patch v2 - With API test
Comment on attachment 132690 [details] Patch v2 - With API test It just occurred to me that you can make it even simpler and just change PlatformPasteboard::setStringForType and hide that logic in there. But I'm fine either way.
In case you're wondering the failure gtk-ews reported for the first patch is bogus.
(In reply to comment #11) > (From update of attachment 132690 [details]) > It just occurred to me that you can make it even simpler and just change PlatformPasteboard::setStringForType and hide that logic in there. But I'm fine either way. *LOL* YUP. I'm going to do it that way.
(In reply to comment #12) > In case you're wondering the failure gtk-ews reported for the first patch is bogus. It appeared to be... thanks for confirming!
Enrica reviewing the greatly simplified patch spawned from her suggestion on IRC. Landed in http://trac.webkit.org/changeset/111267