[webkit-reviews] review denied: [Bug 51013] [Chromium] Should implement EditorClientImpl::requestCheckingOfString() : [Attachment 76627] rebased to ToT
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 15 12:44:58 PST 2010
Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 51013: [Chromium] Should implement
EditorClientImpl::requestCheckingOfString()
https://bugs.webkit.org/show_bug.cgi?id=51013
Attachment 76627: rebased to ToT
https://bugs.webkit.org/attachment.cgi?id=76627&action=review
------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=76627&action=review
> WebKit/chromium/public/WebTextCheckingResult.h:43
> + static WebTextCheckingResult misspellingAt(int location, int length);
> + static WebTextCheckingResult badGrammarAt(int location, int length);
these static methods need the WEBKIT_API prefix so that they can be exported
from WebKit when it is built as a DLL.
please review:
http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg13648.html
> WebKit/chromium/public/WebTextCheckingResult.h:46
> + int type() const { return m_type; }
it is generally better to define an enum for things like this. then add code
to
AssertMatchingEnums.cpp to ensure that the API defined enums do not get out of
sync
with the WebCore defined ones.
> WebKit/chromium/public/WebTextCheckingResult.h:47
> + int location() const { return m_location; }
nit: i think it is more common to use the term "position" or "offset" in cases
like this.
> WebKit/chromium/public/WebViewClient.h:177
> + // WebView::respondCheckingOfString()
nit: this comment is incorrect
More information about the webkit-reviews
mailing list