third_party/WebKit/Source/core/editing/SelectionController.cpp - Issue 2931443003: Add support for Android spellcheck menu in Chrome/WebViews

Unified Diff: third_party/WebKit/Source/core/editing/SelectionController.cpp

Issue 2931443003: Add support for Android spellcheck menu in Chrome/WebViews (Closed)

Patch Set: Respond to aelias@'s comments Created 3 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« content/public/android/java/strings/android_content_strings.grd ('K') | « third_party/WebKit/Source/core/editing/BUILD.gn ('k') | third_party/WebKit/Source/core/editing/suggestion/TextSuggestionBackendImpl.h » ('j') | third_party/WebKit/Source/core/editing/suggestion/TextSuggestionBackendImpl.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: third_party/WebKit/Source/core/editing/SelectionController.cpp

diff --git a/third_party/WebKit/Source/core/editing/SelectionController.cpp b/third_party/WebKit/Source/core/editing/SelectionController.cpp

index 64774b9ae378fde81561d31b2c985535acf4cecb..09d68f7f91c7a965b3e1721194ee504da9df6cfb 100644

--- a/third_party/WebKit/Source/core/editing/SelectionController.cpp

+++ b/third_party/WebKit/Source/core/editing/SelectionController.cpp

@@ -37,6 +37,7 @@

#include "core/editing/RenderedPosition.h"

#include "core/editing/iterators/TextIterator.h"

#include "core/editing/markers/DocumentMarkerController.h"

+#include "core/editing/suggestion/TextSuggestionController.h"

#include "core/events/Event.h"

#include "core/frame/LocalFrame.h"

#include "core/frame/LocalFrameView.h"

@@ -279,6 +280,7 @@ bool SelectionController::HandleSingleClick(

kCharacterGranularity,

is_handle_visible ? HandleVisibility::kVisible

: HandleVisibility::kNotVisible);

+ frame_->GetTextSuggestionController().HandlePotentialMisspelledWordTap();

yosin_UTC9 2017/06/20 01:46:36 Do we really want to ask spell checker for every s

rlanday 2017/06/20 05:34:21 To clarify, we're not calling the spell checker he

Xiaocheng 2017/06/20 05:55:04 If we move the triggering of spellcheck menu to so

If we move the triggering of spellcheck menu to somewhere asynchronous, is it going to introduce weird interaction? For example, after tapping on a misspelled word, the normal selection menu appears, and then (after 300ms) gets replaced by the spellcheck menu? I think we should do some experiments to see the performance impact. It's more convincing to talk about performance with real data. Those long wikipedia articles' editing pages are good benchmarks.

yosin_UTC9 2017/06/20 06:47:21 Can we tell spelling marker information to selecti

On 2017/06/20 at 05:55:04, Xiaocheng wrote: > On 2017/06/20 at 05:34:21, rlanday wrote: > > On 2017/06/20 at 01:46:36, yosin_UTC9 wrote: > > > Do we really want to ask spell checker for every single click? > > > This change makes mouse handling slow down. > > > > > > I think we should stop calling |HandlePotentialMisspelledWordTap()| when "selectstart" is canceled > > > which is checked in |UpdateSelectionForMouseDownDispatchingSelectStart()| just before this call. > > > > > > Can we move this to idle callback like idle time spell checker? > > > > To clarify, we're not calling the spell checker here, we're just checking for the presence of a spelling DocumentMarker. > > > > I'm kind of hesitant to make this asynchronous because we want to show the popup menu 300ms (the double-tap threshold) after the tap, and we can't start the timer until after HandlePotentialMisspelledWordTap() runs. So if we make this run in idle-time, the timing will probably get thrown off and the interaction will feel laggy. > > > > One tradeoff we can make is, instead of creating the two-character wide selection range on every tap and checking for a DocumentMarker in that range (which requires using TextIterator, which is a somewhat complex operation), we can start the timer on every tap, and only check for the spelling marker if the timer expires. That way, if we get a bunch of touch events with less than 300ms between them, we only have to check for the spelling marker after the last one. What do you think of this idea? The tradeoff would be that we have to reset the timer more often. > > If we move the triggering of spellcheck menu to somewhere asynchronous, is it going to introduce weird interaction? For example, after tapping on a misspelled word, the normal selection menu appears, and then (after 300ms) gets replaced by the spellcheck menu? > > I think we should do some experiments to see the performance impact. It's more convincing to talk about performance with real data. Those long wikipedia articles' editing pages are good benchmarks.

Can we tell spelling marker information to selection menu popup menu? It seems popup menu queries state of Copy, Paste, etc, we can tell marker information at that time. BTW, I withdraw idle-time-callback idea, it doesn't solve inconsistent menu issue.

return false;

}