Bug 88618 – [chromium] Use the marker region to replace a misspelled word with a suggestion

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Hironori Bono]

Greetings,

Our new spellchecker adds markers to wrongly-separated words. For example, when we type a sentence "dive in to the rabbit hole", it adds a marker to 'in to' and adds a suggestion 'into'. Unfortunately, when we choose the suggestion "into", Chromium replaces only the word selected by caret with the suggestion. (For this case, it replaces 'in' or 'to' with 'into'.) To fix this problem, I would like to change Chromium to use a marker region to replace a misspelled region.

Regards,

Hironori Bono

[Hironori Bono]

Created attachment 146482 [details]
Patch v0

Greetings,

I have quickly implemented a function that replaces the range of a misspelled marker with text. Would it be possible to review this change?

Regards,

Hironori Bono

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Ryosuke Niwa]

Comment on attachment 146482 [details]
Patch v0

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

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1363
> +    // If this caret selection has two or more markers, this function replace the range covered by the first marker with the specified word as Microsoft Word does.
> +    if (pluginContainerFromFrame(frame()))
> +        return;
> +    RefPtr<Range> caretRange = frame()->selection()->toNormalizedRange();
> +    if (!caretRange)
> +        return;
> +    Vector<DocumentMarker*> markers = frame()->document()->markers()->markersInRange(caretRange.get(), DocumentMarker::Spelling | DocumentMarker::Grammar);
> +    if (markers.size() < 1 || markers[0]->startOffset() >= markers[0]->endOffset())
> +        return;
> +    RefPtr<Range> markerRange = TextIterator::rangeFromLocationAndLength(frame()->selection()->rootEditableElementOrDocumentElement(), markers[0]->startOffset(), markers[0]->endOffset() - markers[0]->startOffset());
> +    if (!markerRange.get() || !frame()->selection()->shouldChangeSelection(markerRange.get()))
> +        return;
> +    frame()->selection()->setSelection(markerRange.get(), CharacterGranularity);
> +    frame()->editor()->replaceSelectionWithText(text, false, true);

This code looks awfully familiar. In fact, I saw almost exactly same comment elsewhere.

[Hajime Morrita]

Comment on attachment 146482 [details]
Patch v0

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

>> Source/WebKit/chromium/src/WebFrameImpl.cpp:1363
>> +    frame()->editor()->replaceSelectionWithText(text, false, true);
> 
> This code looks awfully familiar. In fact, I saw almost exactly same comment elsewhere.

Searched with replaceSelectionWithText(), but I couldn't find any. 
It looks Mac port (and other ports) does this in WebKit layer for obeying AppKit protocol or whatever reason.

I hope this core logic to be a Editor method to allow future refactoring though.

[Hironori Bono]

Greetings Niwa-san,

Thanks for your comment. To use code search, my code seems to be a subset of AlternativeTextController::handleAlternativeTextUIResult() <http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/editing/AlternativeTextController.cpp&exact_package=chromium&q=handleAlternativeTextUIResult&type=cs&l=354> in terms of functionality. (This function is for the auto-correction panel of Mac and not used by Chromium, though.)

Regards,

Hironori Bono

(In reply to comment #3)
> This code looks awfully familiar. In fact, I saw almost exactly same comment elsewhere.

[Ryosuke Niwa]

Alexey and Enrica might know where this code is located in Mac port.

[Hironori Bono]

Greetings Niwa-san,

For what it's worth, I wonder how I should treat this change. Is it better to move this code to WebCore (Editor), or to keep it in the WebKit layer?

Regards,

Hironori Bono

(In reply to comment #6)
> Alexey and Enrica might know where this code is located in Mac port.

[Kent Tamura]

Comment on attachment 146482 [details]
Patch v0

Looks ok.  We can share code later if one wants to do so.

[WebKit Review Bot]

Comment on attachment 146482 [details]
Patch v0

Clearing flags on attachment: 146482

Committed r120274: <http://trac.webkit.org/changeset/120274>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.