Bug 82497

Summary: REGRESSION: editing/pasteboard/dataTransfer-setData-getData.html fails on Mac platforms
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: HTML EditingAssignee: Enrica Casucci <enrica>
Severity: Normal CC: beidson, webkit-bug-importer
Priority: P2 Keywords: InRadar, LayoutTestFailure, MakingBotsRed
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: Unspecified   
Description Flags
beidson: review-
Patch2 beidson: review+

Description Enrica Casucci 2012-03-28 12:02:46 PDT
This is very likely a regression caused by http://trac.webkit.org/changeset/111267.
This revision fixed the way data is written to the pasteboard for NSURLPboardType.
We now need to fix how it is read from the pasteboard in that format.
Comment 1 Radar WebKit Bug Importer 2012-03-28 12:03:14 PDT
Comment 2 Enrica Casucci 2012-03-28 12:23:08 PDT
Created attachment 134367 [details]
Comment 3 Brady Eidson 2012-03-28 12:30:52 PDT
Comment on attachment 134367 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=134367&action=review

> Source/WebCore/platform/mac/ClipboardMac.mm:207
>      // Fallback to NSURLPboardType (which is a single URL)
>      if (availableTypes.contains(String(NSURLPboardType))) {
> -        platformStrategies()->pasteboardStrategy()->getPathnamesForType(absoluteURLs, String(NSURLPboardType), pasteboardName);
> +        absoluteURLs.append(platformStrategies()->pasteboardStrategy()->stringForType(String(NSURLPboardType), pasteboardName));
>          if (!absoluteURLs.isEmpty())
>              return absoluteURLs;

It seems that the old code - using getPathnamesForType() - could return anywhere from 0 to many URLs so the absoluteURLs.isEmpty() check was relevant.

But the comment suggests that NSURLPboardType should only ever be 1 URL.  And - in fact - this new code can only ever return exactly 1 URL.

Nuke that unnecessary !absoluteURLs.isEmpty() check!  :)
Comment 4 Brady Eidson 2012-03-28 12:32:06 PDT
And also - did TestWebKitAPI run successfully with this change?
Comment 5 Enrica Casucci 2012-03-28 12:45:17 PDT
Created attachment 134373 [details]

New patch that addresses reviewer's comments.
Comment 6 Enrica Casucci 2012-03-28 16:43:46 PDT
Comment 7 Alexey Proskuryakov 2012-03-28 17:08:40 PDT
Isn't this the same as bug 81125 and its associated radar?
Comment 8 Brady Eidson 2012-03-28 17:24:11 PDT
(In reply to comment #7)
> Isn't this the same as bug 81125 and its associated radar?

Seems almost certain that it is