Bug 73799

Summary: Update String::containsOnlyASCII() to handle 8 bits strings
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: Web Template FrameworkAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, jamesr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch darin: review+

Benjamin Poulain
Reported 2011-12-04 22:34:40 PST
Update String::containsOnlyASCII() to handle 8 bits strings
Attachments
Patch (5.80 KB, patch)
2011-12-04 22:46 PST, Benjamin Poulain
no flags
Patch (6.56 KB, patch)
2011-12-05 15:18 PST, Benjamin Poulain
darin: review+
Benjamin Poulain
Comment 1 2011-12-04 22:46:08 PST
Sam Weinig
Comment 2 2011-12-05 10:40:03 PST
Comment on attachment 117845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117845&action=review > Source/JavaScriptCore/wtf/text/WTFString.h:504 > + if (is8Bit()) { > + const LChar* characters = characters8(); > + LChar ored = 0; > + for (size_t i = 0; i < m_impl->length(); ++i) > + ored |= characters[i]; > + return !(ored & 0x80); > + } It would probably make sense to add a charactersAreAllASCII that takes LChars, to match the one that takes UChars, and call it here.
Benjamin Poulain
Comment 3 2011-12-05 15:18:59 PST
Darin Adler
Comment 4 2011-12-05 15:22:14 PST
Comment on attachment 117946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117946&action=review > Source/JavaScriptCore/wtf/text/WTFString.h:492 > return !(ored & 0xFF80); This is the only thing here that is specific to 2 bytes. It could be written like this: return !(ored & ~0x7F); And then it would only require that CharType was the size of an integer or smaller. Or it could be written like this: CharType lowBits = 0x7F; return !(ored & ~lowBits); And then it would work with pretty much any type and a COMPILE_ASSERT would not be needed.
Benjamin Poulain
Comment 5 2011-12-05 15:25:10 PST
I like that. Thanks for the review.
Benjamin Poulain
Comment 6 2011-12-05 15:58:14 PST
Note You need to log in before you can comment on or make changes to this bug.