Bug 126479

Summary: Add support to retrieve the autocorrection context in WK2
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: WebKit2Assignee: Enrica Casucci <enrica>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch sam: review+

Description Enrica Casucci 2014-01-03 17:48:05 PST
This bug tracks the work required to retrieve the context for autocorrection and IME on iOS.
Comment 1 Enrica Casucci 2014-01-03 17:52:53 PST
Created attachment 220361 [details]
Patch
Comment 2 Sam Weinig 2014-01-03 18:17:13 PST
Comment on attachment 220361 [details]
Patch

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?
Comment 3 Enrica Casucci 2014-01-06 10:52:16 PST
Thanks for the review. I'll address your comments.
> 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/?
This is all new code.
> 
> > 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?.
Sure.
Comment 4 Enrica Casucci 2014-01-06 14:22:36 PST
Committed 161358.