[webkit-reviews] review granted: [Bug 195795] Add a platform-driven spell-checking mechanism : [Attachment 365202] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Tim Horton <thorton at apple.com>'s
request for review:
Bug 195795: Add a platform-driven spell-checking mechanism
https://bugs.webkit.org/show_bug.cgi?id=195795

Attachment 365202: Patch

https://bugs.webkit.org/attachment.cgi?id=365202&action=review




--- Comment #16 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 365202
  --> https://bugs.webkit.org/attachment.cgi?id=365202
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365202&action=review

r=me. I guess we can polish the code more as the related radar gets resolved.

> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:73
> +RefPtr<Range>
TextCheckingControllerProxy::rangeAndOffsetRelativeToSelection(int64_t offset,
uint64_t length, size_t& outLocationInRoot)

Make this return optional<struct { Ref<Range> range; size_t locationInRoot; }>
instead of taking an out argument?

> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:107
> +	   bool restoreSelection = frameSelection.selection().selectionType()
== VisibleSelection::CaretSelection;

Just check frameSelection.selection().isRange()

> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:109
> +	   Editor::replaceRangeForSpellChecking(*range, [annotatedString.string
string]);

Can we make this a regular instance member function? We already have frame
here.