Bug 138039 - [Mac] Use NSString API in QuickLookMac::computeNeedsQuickLookResourceCachingQuirks()
Summary: [Mac] Use NSString API in QuickLookMac::computeNeedsQuickLookResourceCachingQ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Enhancement
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-23 21:32 PDT by Chris Dumez
Modified: 2014-10-24 13:12 PDT (History)
3 users (show)

See Also:

Attachments
Patch (1.74 KB, patch)
2014-10-23 21:44 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (543.77 KB, application/zip)
2014-10-23 22:25 PDT, Build Bot 		no flags Details
Patch (1.64 KB, patch)
2014-10-23 23:03 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2014-10-23 21:32:27 PDT 
Use NSString API in QuickLookMac::computeNeedsQuickLookResourceCachingQuirks() for case-insensitive string comparison instead of converting to UTF-8 and then using strcasecmp().
Comment 1 Chris Dumez 2014-10-23 21:44:43 PDT 
Created attachment 240391 [details]
Patch
Comment 2 Build Bot 2014-10-23 22:25:01 PDT 
Created attachment 240392 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 3 Chris Dumez 2014-10-23 22:33:48 PDT 
Hmm, the following layout tests are failing on the bots, so I must have changed the behavior unintentionally:
Regressions: Unexpected text-only failures (2)
  http/tests/cache/loaded-from-cache-after-reload-within-iframe.html [ Failure ]
  http/tests/cache/post-redirect-get.php [ Failure ]
Comment 4 Chris Dumez 2014-10-23 23:03:46 PDT 
Created attachment 240397 [details]
Patch

Gosh, [NSString caseInsensitiveCompare] returns True if the NSString is nil :)
Comment 5 Darin Adler 2014-10-24 09:17:05 PDT 
Comment on attachment 240397 [details]
Patch

Did you land this already? I would set review+ but it does’t have review?.
Comment 6 Chris Dumez 2014-10-24 10:23:20 PDT 
(In reply to comment #5)
> Comment on attachment 240397 [details]
> Patch
> 
> Did you land this already? I would set review+ but it does’t have review?.

I did not land it. Bugzilla just won't let me set the review flag (gives me an error)...
Comment 7 Chris Dumez 2014-10-24 10:37:33 PDT 
Bugzilla is partially back and the review flag is set!
Comment 8 Darin Adler 2014-10-24 12:34:12 PDT 
Comment on attachment 240397 [details]
Patch

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

> Source/WebCore/platform/mac/QuickLookMac.mm:45
> +        if (bundleID && [bundleID caseInsensitiveCompare:@"com.apple.QuickLookUIFramework"] == NSOrderedSame)

(Side note: This does a little extra work because it for use in sorting, not just an equality comparison, which is why the WebKit framework has a helper method for this that uses a slightly different method. But here this seems fine.)
Comment 9 WebKit Commit Bot 2014-10-24 13:12:43 PDT 
Comment on attachment 240397 [details]
Patch

Clearing flags on attachment: 240397

Committed r175174: <http://trac.webkit.org/changeset/175174>
Comment 10 WebKit Commit Bot 2014-10-24 13:12:46 PDT 
All reviewed patches have been landed.  Closing bug.