[webkit-reviews] review granted: [Bug 126479] Add support to retrieve the autocorrection context in WK2 : [Attachment 220361] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 126479: Add support to retrieve the autocorrection context in WK2
https://bugs.webkit.org/show_bug.cgi?id=126479

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=220361&action=review


> Source/WebKit2/UIProcess/AutoCorrectionCallback.h:118
> +private:
> +
> +    AutocorrectionContextCallback(void* context, CallbackFunction callback)

Exra newline.

> Source/WebKit2/UIProcess/API/ios/WKInteractionView.mm:1643
> +    WKAutocorrectionContext *context =[[WKAutocorrectionContext alloc]
init];

Missing space after the =.

> Source/WebKit2/UIProcess/API/ios/WKInteractionView.mm:1652
> +    if (beforeText && [beforeText length])
> +	   context.contextBeforeSelection = [beforeText copy];
> +    if (selectedText && [selectedText length])
> +	   context.selectedText = [selectedText copy];
> +    if (markedText && [markedText length])
> +	   context.markedText = [markedText copy];
> +    if (afterText && [afterText length])
> +	   context.contextAfterSelection = [afterText copy];

I think calling length when the NSString is nil is fine, no need to nil check
them.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:725
> +static void computeAutocorrectionContext(Frame& frame, String&
contextBefore, String& markedText, String& selectedText, String& contextAfter,
NSRange& selectedRangeInMarkedText)

Is this all new code or is it a version of something elsewhere?  If it is kind
of duplicated, can we move this down into WebCore/?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:792
> +    NSRange selectedRangeInMarkedText;

I don't see a great reason to use NSRange here. Can we just use a pair of
uint64_ts?