Bug 25847 – Clean up ClipboardMac

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Eric Seidel (no email)]

Clean up ClipboardMac

I was in ClipboardMac today and decided to do a little cleanup.

I'm mostly just moving code, however there are 3 changes I made while moving.

1.  I broke out logic into absoluteURLsFromPasteboard which returns an NSArray (different!) so that we can use -[NSArray componentsJoinedByString] which is cleaner, and allows easier access to the full list of file urls from the pasteboard (which is more useful than the string concatenation of them).
2.      unsigned count = (type == "URL") ? 1 : [fileList count]; is now an explicit check for "URL", before it was a check for != "text/uri-list" which was much more confusing in my opinion (since text/uri-list and URL are the only two types which map to NSURLPboardType as far as I can tell.
3.  I removed an extra if (!types) check, right before [types count], in Obj-C messaging nil will return 0 (size of a pointer), so it's safe to message nil here and expect it to return 0.

[Eric Seidel (no email)]

Created attachment 30437 [details]
ClipboardMac cleanup

 2 files changed, 90 insertions(+), 55 deletions(-)

[Darin Adler]

Comment on attachment 30437 [details]
ClipboardMac cleanup

I think it's unnecessary to shout "NOTE".

There's no reason to have a default value for the "onlyFirstURL" boolean.

> +    if (!absoluteURLs) {
> +        if ([availableTypes containsObject:NSURLPboardType]) {

Using && here would avoid an otherwise-unnecessary level of if nesting. But I think early return would be even better. One of the advantages of breaking something into a separate function.

> +        // "URL" and "text/url-list" both map to NSURLPboardType in cocoaTypeFromMIMEType(), "URL" only wants the first URL

It's a little strange to have this same comment twice.

> +    unsigned count = [types count];
> +    for (unsigned i = 0; i < count; i++) {

As long as you are touching this code, the correct type here is NSUInteger rather than unsigned. Same for the other count and index in absoluteURLsFromPasteboardFilenames.

r=me

[Eric Seidel (no email)]

I made all your suggested changes and ran the layout tests again.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/platform/mac/ClipboardMac.mm
Committed r43850

Thanks!

[David Kilzer (:ddkilzer)]

http://trac.webkit.org/changeset/43850

Tiger build fix:

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/platform/mac/ClipboardMac.mm
Committed r43852

http://trac.webkit.org/changeset/43852

[Eric Seidel (no email)]

Thank you David.