[webkit-reviews] review denied: [Bug 27371] [WINCE] Add WinCE specific platform/text files to WebCore : [Attachment 34875] modified
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Aug 24 17:26:20 PDT 2009
Eric Seidel <eric at webkit.org> has denied Yong Li <yong.li at torchmobile.com>'s
request for review:
Bug 27371: [WINCE] Add WinCE specific platform/text files to WebCore
https://bugs.webkit.org/show_bug.cgi?id=27371
Attachment 34875: modified
https://bugs.webkit.org/attachment.cgi?id=34875&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
This line is in the middle of nowhere:
5 * All rights reserved.
Not true:
2 * This file is part of the DOM implementation for KDE.
Why?
246 #undef isSentenceStop
I'm not sure what this syntax does:
59 friend LanguageManager& languageManager();
Why std::map instead of HashMap?
static std::map<UINT, std::string>& codePageCharsets()
65 {
66 static std::map<UINT, std::string> cc;
67 return cc;
68 }
Indent:
214 TextCodecWince::TextCodecWince(const TextEncoding& encoding)
215 : m_encoding(encoding)
Comments are likely needed here:
231 if (codePage > 50000) {
232 if ((codePage >= 50220 && codePage <= 50222)
233 || codePage == 50225
234 || codePage == 50227
235 || codePage == 50229
236 || codePage == 52936
237 || codePage == 54936
238 || (codePage >= 57002 && codePage <= 57001)
239 || codePage == 65000 // UTF-7
We generally use full english words for variable names:
293 int rtn = encMbToWc(dst, (const unsigned
char*)bytes, srcEnd - bytes);
294 if (rtn >= 0)
(obviously we may have to adjust to avoid keywords here)
I dont' think explicit construction is needed here:
66 if (resultLength <= 0)
467 return CString("?");
r-, mostly needing explanation as to why the std:: types, generally we only use
std:: algorithms, not types in WebCore. IIRC that's due to compiling without
exceptions.
More information about the webkit-reviews
mailing list