| Summary: | Fix recently-introduced off-by-one error in centerTruncateToBuffer | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||
| Component: | WebKit Misc. | Assignee: | Darin Adler <darin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | ap, conrad_shultz | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Darin Adler
2014-03-27 23:42:35 PDT
I finished the fix about an hour ago, but I decided to add unit tests for WebStringTruncator, which is taking me a little time. Created attachment 228030 [details]
Patch
I originally thought this was Mac-only, but it seems this code is used on other platforms as well. Comment on attachment 228030 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228030&action=review > Source/WebKit/mac/ChangeLog:12 > + (+[WebStringTruncator initialize]): Added threading initialization, needed for main > + thread assertions in string truncator code. I'm confused about how this worked before - you didn't add any new assertions in this patch. Is something like this needed on iOS too? Would it be worth adding some non-ASCII inputs to the unit tests? Comment on attachment 228030 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228030&action=review > Source/WebKit/mac/Misc/WebStringTruncator.mm:67 > +#if PLATFORM(MAC) As Alexey and I discussed, we don’t need this #if since this file is already Mac-only (has NSFont). Committed r166448: <http://trac.webkit.org/changeset/166448> The regression test fails on Mountain Lion: /Volumes/Data/slave/mountainlion-debug/build/Tools/TestWebKitAPI/Tests/mac/StringTruncator.mm:36: Failure Value of: "abcdef…tuvwxyz" Expected: [[WebStringTruncator centerTruncateString:@"abcdefghijklmnopqrstuvwxyz" toWidth:100 withFont:[NSFont fontWithName:@"Helvetica" size:12]] UTF8String] Which is: "abcdefg…tuvwxyz" Updated expectation for 10.8 in <http://trac.webkit.org/r166458>. |