[webkit-reviews] review denied: [Bug 44114] [Qt] Missing spell check support : [Attachment 103905] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Lindsay Mathieson
<lindsay.mathieson at gmail.com>'s request for review:
Bug 44114: [Qt] Missing spell check support
https://bugs.webkit.org/show_bug.cgi?id=44114

Attachment 103905: Patch
https://bugs.webkit.org/attachment.cgi?id=103905&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103905&action=review


> Source/WebCore/ChangeLog:9
> +	   No new tests. (OOPS!)

You should removing this.

> Source/WebCore/ChangeLog:29
> +2011-08-12  Lindsay Mathieson  <lindsay.mathieson at gmail.com> and Dawit
Alemayehu   <adawit at kde.org>
> +
> +	   [Qt] Missing spell check support
> +	   https://bugs.webkit.org/show_bug.cgi?id=44114
> +	   Implements platform plugin  api for spell and grammar checking,
> +	   draws error lines for same.
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   No new tests. (OOPS!)
> +
> +	   * platform/graphics/qt/GraphicsContextQt.cpp:
> +	   (WebCore::drawErrorUnderline):
> +	   (WebCore::GraphicsContext::drawLineForTextChecking):
> +

Two changelogs :(

> Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:943
> +    // Bottom of squiggle

Period at the end.

> Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:959
> +    // Top of squiggle

Period at the end.

> Source/WebKit/qt/Api/qwebkitplatformplugin.h:145
> +class QWEBKIT_EXPORT QWebSpellCheckerGrammarDetail {
> +public:
> +    int location;
> +    int length;
> +    QStringList guesses;
> +    QString userDescription;
> +};

This should go away now that you have QWebSpellChecker::GrammarDetail

> Source/WebKit/qt/ChangeLog:35
> +2011-08-12  Lindsay Mathieson  <lindsay.mathieson at gmail.com>
> +
> +	   [Qt] Missing spell check support
> +	   https://bugs.webkit.org/show_bug.cgi?id=44114
> +	   Fixed style failure.
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   * Api/qwebkitplatformplugin.h:
> +	   * QtWebKit.pro:
> +	   * WebCoreSupport/EditorClientQt.cpp:
> +	   (WebCore::EditorClientQt::isContinuousSpellCheckingEnabled):
> +	   (WebCore::EditorClientQt::isGrammarCheckingEnabled):
> +	   (WebCore::EditorClientQt::toggleContinuousSpellChecking):
> +	   (WebCore::EditorClientQt::toggleGrammarChecking):
> +	   * WebCoreSupport/EditorClientQt.h:
> +	   (WebCore::EditorClientQt::textChecker):
> +	   * WebCoreSupport/QtPlatformPlugin.cpp:
> +	   (WebCore::QtPlatformPlugin::createSpellChecker):
> +	   * WebCoreSupport/QtPlatformPlugin.h:
> +	   * WebCoreSupport/TextCheckerClientQt.cpp: Added.
> +	   (WebCore::TextCheckerClientQt::ignoreWordInSpellDocument):
> +	   (WebCore::TextCheckerClientQt::learnWord):
> +	  
(WebCore::TextCheckerClientQt::getAutoCorrectSuggestionForMisspelledWord):
> +	   (WebCore::TextCheckerClientQt::checkSpellingOfString):
> +	   (WebCore::TextCheckerClientQt::checkGrammarOfString):
> +	   (WebCore::TextCheckerClientQt::getGuessesForWord):
> +	   (WebCore::TextCheckerClientQt::isContinousSpellCheckingEnabled):
> +	   (WebCore::TextCheckerClientQt::toggleContinousSpellChecking):
> +	   (WebCore::TextCheckerClientQt::isGrammarCheckingEnabled):
> +	   (WebCore::TextCheckerClientQt::toggleGrammarChecking):
> +	   (WebCore::TextCheckerClientQt::loadSpellChecker):
> +	   * WebCoreSupport/TextCheckerClientQt.h: Added.
> +	   (WebCore::TextCheckerClientQt::requestCheckingOfString):
> +

Double changelog :(

> Source/WebKit/qt/QtWebKit.pro:198
> +    $$PWD/WebCoreSupport/WebPlatformStrategies.cpp \
> +    $$PWD/WebCoreSupport/TextCheckerClientQt.cpp

Order of the file -> alphabetical?

> Source/WebKit/qt/QtWebKit.pro:215
> +    $$PWD/WebCoreSupport/WebPlatformStrategies.h \
> +    $$PWD/WebCoreSupport/TextCheckerClientQt.h

Order of the file -> alphabetical?