Summary: | Clean up ClipboardMac | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ddkilzer, sam | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Eric Seidel (no email)
2009-05-17 22:30:33 PDT
Created attachment 30437 [details]
ClipboardMac cleanup
2 files changed, 90 insertions(+), 55 deletions(-)
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 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! 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 Thank you David. |