[webkit-reviews] review denied: [Bug 92160] [BlackBerry] Support async spellcheck for the blackberry port : [Attachment 154142] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jul 24 14:51:13 PDT 2012
Rob Buis <rwlbuis at gmail.com> has denied Nima Ghanavatian
<nima.ghanavatian at gmail.com>'s request for review:
Bug 92160: [BlackBerry] Support async spellcheck for the blackberry port
https://bugs.webkit.org/show_bug.cgi?id=92160
Attachment 154142: Patch
https://bugs.webkit.org/attachment.cgi?id=154142&action=review
------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=154142&action=review
Looks good, still some stuff to fix.
> Source/WebKit/blackberry/ChangeLog:54
> + (WebCore):
I think something went wrong with the ChangeLogs, one should detail the
WebCore/ChangeLog changes, the other the changes in
WebKit/blackberry/ChangeLog. The git commit message should do the combination
of the two like above.
> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.cpp:43
> +class SpellChecker;
You don't need this.
> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.cpp:575
> +void EditorClientBlackBerry::checkTextOfParagraph(const UChar*, int,
WebCore::TextCheckingTypeMask, WTF::Vector<WebCore::TextCheckingResult>&)
You don't need some of the prefixes here, like WTF:: for Vector.
> Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.h:88
> + virtual void checkTextOfParagraph(const UChar*, int,
WebCore::TextCheckingTypeMask, WTF::Vector<WebCore::TextCheckingResult>&);
You don't need some of the prefixes here, like WTF:: for Vector.
> Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp:630
> + if (frame) {
You can combine above two lines.
> Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp:632
> + if (editor)
Ditto.
More information about the webkit-reviews
mailing list