Bug 130889 – Fix recently-introduced off-by-one error in centerTruncateToBuffer

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

Fix recently-introduced off-by-one error in centerTruncateToBuffer

[Darin Adler]

I finished the fix about an hour ago, but I decided to add unit tests for WebStringTruncator, which is taking me a little time.

[Darin Adler]

Created attachment 228030 [details]
Patch

[Darin Adler]

I originally thought this was Mac-only, but it seems this code is used on other platforms as well.

[Alexey Proskuryakov]

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?

[Conrad Shultz]

Would it be worth adding some non-ASCII inputs to the unit tests?

[Darin Adler]

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).

[Darin Adler]

Committed r166448: <http://trac.webkit.org/changeset/166448>

[Alexey Proskuryakov]

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"

[Alexey Proskuryakov]

Updated expectation for 10.8 in <http://trac.webkit.org/r166458>.