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
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
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
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.
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.
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.
Alexey and Enrica might know where this code is located in Mac port.
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.
Comment on attachment 146482 [details] Patch v0 Looks ok. We can share code later if one wants to do so.
Comment on attachment 146482 [details] Patch v0 Clearing flags on attachment: 146482 Committed r120274: <http://trac.webkit.org/changeset/120274>
All reviewed patches have been landed. Closing bug.