[webkit-reviews] review granted: [Bug 57862] WebKit2: Implement TextChecker on Windows : [Attachment 88719] Patch (Part 4)

[bugzilla-daemon at webkit.org]

Brian Weinstein <bweinstein at apple.com> has granted Jessie Berlin
<jberlin at webkit.org>'s request for review:
Bug 57862: WebKit2: Implement TextChecker on Windows
https://bugs.webkit.org/show_bug.cgi?id=57862

Attachment 88719: Patch (Part 4)
https://bugs.webkit.org/attachment.cgi?id=88719&action=review

------- Additional Comments from Brian Weinstein <bweinstein at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88719&action=review

> Source/WebKit2/Shared/APIObject.h:110
> +	   TypeGrammarDetail

Is there any sorting to these? If so, GrammarDetail should go above
TextChecker.

> Source/WebKit2/UIProcess/API/C/win/WKTextChecker.h:44
> +typedef void (*WKTextcheckerCheckGrammarOfString)(uint64_t tag, WKStringRef
text, WKArrayRef* grammarDetails, int32_t* badGrammarLocation, int32_t*
badGrammarLength, const void *clientInfo);

Missing a capital C in WKTextcheckerCheckGrammarOfString.

> Source/WebKit2/UIProcess/API/C/win/WKTextChecker.h:57
> +    WKTextcheckerCheckGrammarOfString				      
checkGrammarOfString;

Ditto about missing capital C.

> Source/WebKit2/UIProcess/mac/TextCheckerMac.mm:307
> +    notImplemented();

A note saying where Mac implements this would be nice, since it seems like the
Mac could implement it here.

> Source/WebKit2/UIProcess/win/WebTextCheckerClient.cpp:102
> +{

Do you want to make sure that grammarDetails is empty when it is passed in? Or
does it not matter?

> Source/WebKit2/UIProcess/win/WebTextCheckerClient.cpp:103
> +    if (!m_client.checkSpellingOfString)

This should probably be if (!m_client.checkGrammarOfString)

> Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:420
> +    int32_t resultLocation = -1;

Could we use the NotFound value in WTF/NotFound.h for this?