New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 715365 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task

Blocking:
issue 727172



Sign in to add a comment

Implement Android spellcheck menu

Project Member Reported by rlanday@chromium.org, Apr 26 2017

Issue description

Currently, tapping on a misspelled word does not show the spellcheck
suggestion menu with the "Add to Dictionary" button you get in a native
textbook since it's not implemented in Chrome or WebView. We want to
implement this.
 

Comment 1 by yosin@chromium.org, Apr 27 2017

Labels: -Type-Bug Type-Task
Project Member

Comment 2 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ee9280525895b4dbb74d3e6117547f79b96d89e

commit 8ee9280525895b4dbb74d3e6117547f79b96d89e
Author: rlanday <rlanday@chromium.org>
Date: Fri May 19 21:53:51 2017

Only create RenderedDocumentMarkers for TextMatchMarkerListImpl

This sets us up to start creating a polymorphic class hierarchy for
DocumentMarker. RenderedDocumentMarker will become TextMatchMarker, and we will
also add subclasses of DocumentMarker for the other MarkerTypes as well.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2883503004
Cr-Commit-Position: refs/heads/master@{#473345}

[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/8ee9280525895b4dbb74d3e6117547f79b96d89e/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h

Oops, that CL actually belongs on  bug 707867 
Project Member

Comment 4 by bugdroid1@chromium.org, May 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c45f50742410a644a054325478d885e61d286713

commit c45f50742410a644a054325478d885e61d286713
Author: rlanday <rlanday@chromium.org>
Date: Fri May 26 18:07:06 2017

Allow storing multiple replacements on SpellCheckResult

I'm working on adding support for the Android spellcheck suggestion list that
appears in a native text box when you tap on a misspelled word. It appears that
I need to modify this piece of infrastructure to support passing multiple
suggestions through to the Spelling marker that gets added.

For reference, we construct the SpellCheckResult objects from the Android
spellchecker results in SpellCheckerSessionBridge::ProcessSpellCheckResults().

Alternatively, ContextMenuClientImpl has logic for calling the spellchecker
again to get the list of suggestions that we might be able to use:
https://chromium.googlesource.com/chromium/src/+/adc8910776c64a876f0422454da618a37d01167b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#346

However, I talked to aelias and he said it would be better to store the
suggestions the first time we call the spellchecker, at the time we add the
marker, to avoid going back and forth unnecessarily.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2848943002
Cr-Commit-Position: refs/heads/master@{#475058}

[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/chrome/browser/renderer_context_menu/spelling_menu_observer.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac_unittest.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/chrome/browser/spellchecker/spellcheck_message_filter_unittest.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/chrome/browser/spellchecker/spelling_service_client_unittest.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/common/spellcheck_messages.h
[add] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/common/spellcheck_result.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/common/spellcheck_result.h
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/renderer/spellcheck_provider_test.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/renderer/spellcheck_provider_unittest.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/components/spellcheck/renderer/spellcheck_unittest.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/content/shell/test_runner/mock_spell_check.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/content/shell/test_runner/spell_check_client.cc
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.h
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/Source/platform/text/TextChecking.h
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/Source/web/WebTextCheckingResult.cpp
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/c45f50742410a644a054325478d885e61d286713/third_party/WebKit/public/web/WebTextCheckingResult.h

Comment 5 by noel@chromium.org, May 29 2017

Blocking: 727172
Project Member

Comment 6 by bugdroid1@chromium.org, May 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02

commit e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02
Author: noel <noel@chromium.org>
Date: Mon May 29 05:54:15 2017

Revert of Allow storing multiple replacements on SpellCheckResult (patchset #11 id:200001 of https://codereview.chromium.org/2848943002/ )

Reason for revert:
Caused spell markers to no longer appear on OSX, per  crbug.com/727172 

Original issue's description:
> Allow storing multiple replacements on SpellCheckResult
>
> I'm working on adding support for the Android spellcheck suggestion list that
> appears in a native text box when you tap on a misspelled word. It appears that
> I need to modify this piece of infrastructure to support passing multiple
> suggestions through to the Spelling marker that gets added.
>
> For reference, we construct the SpellCheckResult objects from the Android
> spellchecker results in SpellCheckerSessionBridge::ProcessSpellCheckResults().
>
> Alternatively, ContextMenuClientImpl has logic for calling the spellchecker
> again to get the list of suggestions that we might be able to use:
> https://chromium.googlesource.com/chromium/src/+/adc8910776c64a876f0422454da618a37d01167b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#346
>
> However, I talked to aelias and he said it would be better to store the
> suggestions the first time we call the spellchecker, at the time we add the
> marker, to avoid going back and forth unnecessarily.
>
> BUG= 715365 
>
> Review-Url: https://codereview.chromium.org/2848943002
> Cr-Commit-Position: refs/heads/master@{#475058}
> Committed: https://chromium.googlesource.com/chromium/src/+/c45f50742410a644a054325478d885e61d286713

TBR=jochen@chromium.org,aelias@chromium.org,groby@chromium.org,mkwst@chromium.org,xiaochengh@chromium.org,rlanday@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 715365 ,  727172 

Review-Url: https://codereview.chromium.org/2906243002
Cr-Commit-Position: refs/heads/master@{#475293}

[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/chrome/browser/renderer_context_menu/spelling_menu_observer.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac_unittest.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/chrome/browser/spellchecker/spellcheck_message_filter_unittest.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/chrome/browser/spellchecker/spelling_service_client_unittest.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/common/spellcheck_messages.h
[delete] https://crrev.com/64b85971b8b58c052b7e8b53754d5558b8e1dc53/components/spellcheck/common/spellcheck_result.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/common/spellcheck_result.h
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/renderer/spellcheck_provider_test.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/renderer/spellcheck_provider_unittest.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/components/spellcheck/renderer/spellcheck_unittest.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/content/shell/test_runner/mock_spell_check.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/content/shell/test_runner/spell_check_client.cc
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.h
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/Source/platform/text/TextChecking.h
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/Source/web/WebTextCheckingResult.cpp
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/e6a7d7e3cd5665b6bc9b0e8cac4c813c52f90b02/third_party/WebKit/public/web/WebTextCheckingResult.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/455b156778ed1f2df63f8a14dff99399c88e475c

commit 455b156778ed1f2df63f8a14dff99399c88e475c
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 02 14:48:11 2017

[Reland] Allow storing multiple replacements on SpellCheckResult

This CL was reverted because it did not properly handle the case where a
misspelling result is created with no suggested replacements. We want to
preserve misspelling results that don't include any suggested replacements, but
remove those that have some suggested replacements but they're all just changing
between different types of apostrophes.

Original CL:
https://codereview.chromium.org/2848943002

Revert:
https://codereview.chromium.org/2906243002

Original description:

Allow storing multiple replacements on SpellCheckResult

I'm working on adding support for the Android spellcheck suggestion list that
appears in a native text box when you tap on a misspelled word. It appears that
I need to modify this piece of infrastructure to support passing multiple
suggestions through to the Spelling marker that gets added.

For reference, we construct the SpellCheckResult objects from the Android
spellchecker results in SpellCheckerSessionBridge::ProcessSpellCheckResults().

Alternatively, ContextMenuClientImpl has logic for calling the spellchecker
again to get the list of suggestions that we might be able to use:
https://chromium.googlesource.com/chromium/src/+/adc8910776c64a876f0422454da618a37d01167b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#346

However, I talked to aelias and he said it would be better to store the
suggestions the first time we call the spellchecker, at the time we add the
marker, to avoid going back and forth unnecessarily.

BUG= 715365 , 727172 

Review-Url: https://codereview.chromium.org/2911253003
Cr-Commit-Position: refs/heads/master@{#476644}

[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/chrome/browser/renderer_context_menu/spelling_menu_observer.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/chrome/browser/spellchecker/spell_check_host_impl_unittest.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/chrome/browser/spellchecker/spellcheck_message_filter_platform_mac_unittest.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/chrome/browser/spellchecker/spelling_service_client_unittest.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/common/BUILD.gn
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/common/spellcheck_messages.h
[add] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/common/spellcheck_result.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/common/spellcheck_result.h
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/renderer/spellcheck.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/renderer/spellcheck_provider_test.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/renderer/spellcheck_provider_unittest.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/components/spellcheck/renderer/spellcheck_unittest.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/content/shell/test_runner/mock_spell_check.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/content/shell/test_runner/spell_check_client.cc
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.h
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/Source/core/exported/WebTextCheckingResult.cpp
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/Source/platform/text/TextChecking.h
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/455b156778ed1f2df63f8a14dff99399c88e475c/third_party/WebKit/public/web/WebTextCheckingResult.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7

commit 44d1353d9f4b134e7aee2b1d70eae0e0320b42c7
Author: rlanday <rlanday@chromium.org>
Date: Tue Jun 06 06:58:20 2017

Introduce abstract StyleableMarker subclass of DocumentMarker

This CL refactors some code in CompositionMarker into a new abstract class
StyleableMarker, which will contain shared logic for MarkerTypes that can be
rendered with different underline color/thickness and/or background color.

This is in preparation for adding two new MarkerTypes I need to support some
features on Android (one for marking the range of text an open spellcheck/
suggestion menu pertains to, the other for marking ranges of text the user can
open a suggestion menu for by tapping on them).

BUG= 715365 

Review-Url: https://codereview.chromium.org/2922233002
Cr-Commit-Position: refs/heads/master@{#477221}

[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/CompositionMarker.cpp
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/CompositionMarker.h
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImplTest.cpp
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/CompositionMarkerTest.cpp
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[add] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/StyleableMarker.cpp
[add] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/editing/markers/StyleableMarker.h
[modify] https://crrev.com/44d1353d9f4b134e7aee2b1d70eae0e0320b42c7/third_party/WebKit/Source/core/testing/Internals.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf561e1c071256c42e108ba8fd1fb71dcd1db57b

commit cf561e1c071256c42e108ba8fd1fb71dcd1db57b
Author: rlanday <rlanday@chromium.org>
Date: Wed Jun 07 00:15:37 2017

Add ActiveSuggestionMarker

This is a new type of DocumentMarker that will be used on Android to highlight
a region of text that an open spellcheck/suggestion menu pertains to. For a
spellcheck menu, we'll want to highlight the text in red (to match the
underline), but for a suggestion menu, we'll want to highlight the text in the
color of the suggestion underline (on Android, there are a handful of different
colors used based on which flags are set on the corresponding SuggestionSpan).
So I'm making this marker inherit from the StyleableMarker subclass I
refactored CompositionMarker on top of in
https://codereview.chromium.org/2922233002.

This CL doesn't actually add code to paint ActiveSuggestionMarkers; I will add
that in a future CL (together with layout tests).

BUG= 715365 

Review-Url: https://codereview.chromium.org/2923033002
Cr-Commit-Position: refs/heads/master@{#477476}

[modify] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/BUILD.gn
[add] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarker.cpp
[add] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarker.h
[add] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImpl.cpp
[add] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImpl.h
[add] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImplTest.cpp
[add] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerTest.cpp
[modify] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/DocumentMarker.h
[modify] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/core/editing/markers/StyleableMarker.cpp
[modify] https://crrev.com/cf561e1c071256c42e108ba8fd1fb71dcd1db57b/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8

commit b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8
Author: rlanday <rlanday@chromium.org>
Date: Wed Jun 07 00:21:39 2017

Handle ActiveSuggestionMarkers for accessibility

I'm adding a new type of DocumentMarker, ActiveSuggestionMarker, in
https://codereview.chromium.org/2923033002. This MarkerType will be used to mark
the region of text that an open spellcheck/suggestion menu pertains to. This
seems potentially useful to expose for accessibility, so I'm doing that in this
CL (I didn't want to add more complexity to the original CL).

BUG= 715365 

Review-Url: https://codereview.chromium.org/2931473002
Cr-Commit-Position: refs/heads/master@{#477481}

[modify] https://crrev.com/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8/content/renderer/accessibility/blink_ax_enum_conversion.cc
[modify] https://crrev.com/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
[modify] https://crrev.com/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8/third_party/WebKit/Source/web/AssertMatchingEnums.cpp
[modify] https://crrev.com/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8/third_party/WebKit/public/web/WebAXEnums.h
[modify] https://crrev.com/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8/ui/accessibility/ax_enums.idl
[modify] https://crrev.com/b1d2f5d73ed7f1656733cc55c4abc8c716b47ee8/ui/accessibility/ax_node_data.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff

commit a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff
Author: rlanday <rlanday@chromium.org>
Date: Wed Jun 07 00:26:58 2017

Add painting for ActiveSuggestionMarker

This MarkerType was added in https://codereview.chromium.org/2923033002, but
that CL did not include code to actually paint this new type of marker. This CL
makes ActiveSuggestionMarkers be painted the same way as CompositionMarkers and
adds layout tests (copied from the existing ones for CompositionMarker) to
verify this.

BUG= 715365 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2925543003
Cr-Commit-Position: refs/heads/master@{#477485}

[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/editing/active-suggestion-marker-basic.html
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/editing/active-suggestion-marker-split.html
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/linux/editing/active-suggestion-marker-basic-expected.png
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/linux/editing/active-suggestion-marker-basic-expected.txt
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/linux/editing/active-suggestion-marker-split-expected.png
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/linux/editing/active-suggestion-marker-split-expected.txt
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/mac/editing/active-suggestion-marker-basic-expected.png
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/mac/editing/active-suggestion-marker-basic-expected.txt
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/mac/editing/active-suggestion-marker-split-expected.png
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/mac/editing/active-suggestion-marker-split-expected.txt
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/win/editing/active-suggestion-marker-basic-expected.png
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/win/editing/active-suggestion-marker-basic-expected.txt
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/win/editing/active-suggestion-marker-split-expected.png
[add] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/LayoutTests/platform/win/editing/active-suggestion-marker-split-expected.txt
[modify] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
[modify] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/a1166ddba7050dd957ca5e0f5e14ce05afd4b0ff/third_party/WebKit/Source/core/testing/Internals.idl

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba70234d44521a171182fb16ce0e2935fd61e2f4

commit ba70234d44521a171182fb16ce0e2935fd61e2f4
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 09 17:56:00 2017

Store suggestions from Android spellchecker in spelling markers

Currently, when we run the Android spellchecker, we use the results to create
spelling DocumentMarkers to draw the red misspelling underlines, but we discard
the suggestions. This CL passes the suggestions into the SpellCheckResult
objects used to create the DocumentMarkers so we end up with the suggestions on
the markers.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2905003003
Cr-Commit-Position: refs/heads/master@{#478328}

[modify] https://crrev.com/ba70234d44521a171182fb16ce0e2935fd61e2f4/components/spellcheck/browser/android/java/src/org/chromium/components/spellcheck/SpellCheckerSessionBridge.java
[modify] https://crrev.com/ba70234d44521a171182fb16ce0e2935fd61e2f4/components/spellcheck/browser/spellchecker_session_bridge_android.cc
[modify] https://crrev.com/ba70234d44521a171182fb16ce0e2935fd61e2f4/components/spellcheck/browser/spellchecker_session_bridge_android.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a98eaedc0233af64b6e647e3dd377321ee2861c

commit 3a98eaedc0233af64b6e647e3dd377321ee2861c
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 09 19:46:19 2017

Add SpellChecker::GetSpellCheckMarkerTouchingSelection()

This CL makes some changes to SpellChecker that I need to implement the Android
spellcheck menu in Chrome. On most platforms, users can apply spellcheck
suggestions by right-clicking in the interior of a misspelled word (nothing
happens if you right-click at an endpoint) and selecting a replacement. On
Android, the spellcheck menu is triggered by tapping on a word (either in the
interior or at an endpoint). This means we need to do two things (both done in
this CL):

- Expose a way for a caller to determine if the selection (which may be an
  empty caret) is currently touching a spellcheck marker. If some text is
  actually selected, we can just see if any spellcheck markers on the text
  nodes making up the selected text intersect the selection. If we just have a
  caret selection, we need to create a range containing one character on either
  side and do the same operation. This method is being added as
  SpellChecker::GetSpellCheckMarkerTouchingSelection(). (Note: this will
  eventually be useful for suggestion markers as well once those are added, so
  we might want to consider putting this method somewhere else).

- Make SpellChecker::ReplaceMisspelledRange() (which looks for a spellcheck
  marker intersecting the selection) work in the case where we have a caret
  selection touching the spellcheck marker. This is done by using the new
  GetSpellCheckMarkerTouchingSelection() method to find the marker.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2925363002
Cr-Commit-Position: refs/heads/master@{#478380}

[modify] https://crrev.com/3a98eaedc0233af64b6e647e3dd377321ee2861c/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/3a98eaedc0233af64b6e647e3dd377321ee2861c/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.h
[modify] https://crrev.com/3a98eaedc0233af64b6e647e3dd377321ee2861c/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61856d2266ce3b76ac8485a9ad2091d18842472a

commit 61856d2266ce3b76ac8485a9ad2091d18842472a
Author: rlanday <rlanday@chromium.org>
Date: Mon Jun 12 20:53:23 2017

Move spellcheck underline colors into LayoutTheme

I'm adding support in Chrome for the Android spellcheck menu in
https://codereview.chromium.org/2931443003. When a user taps on a misspelled
word, I need to highlight the word in a color based on the color of the
spellcheck underline (we add some transparency). We're trying to avoid
hard-coding the spellcheck color in editing code. Therefore, we think it makes
sense to move the colors for spelling/grammar markers out of
InlineTextBoxPainter into LayoutTheme, so other code can make use of them.

BUG= 715365 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2932913002
Cr-Commit-Position: refs/heads/master@{#478755}

[modify] https://crrev.com/61856d2266ce3b76ac8485a9ad2091d18842472a/third_party/WebKit/Source/core/layout/LayoutTheme.cpp
[modify] https://crrev.com/61856d2266ce3b76ac8485a9ad2091d18842472a/third_party/WebKit/Source/core/layout/LayoutTheme.h
[modify] https://crrev.com/61856d2266ce3b76ac8485a9ad2091d18842472a/third_party/WebKit/Source/core/layout/LayoutThemeMac.h
[modify] https://crrev.com/61856d2266ce3b76ac8485a9ad2091d18842472a/third_party/WebKit/Source/core/layout/LayoutThemeMac.mm
[modify] https://crrev.com/61856d2266ce3b76ac8485a9ad2091d18842472a/third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7fe96812663e16778c2517bd3587eded541088e

commit e7fe96812663e16778c2517bd3587eded541088e
Author: rlanday <rlanday@chromium.org>
Date: Mon Jun 12 22:59:03 2017

Add test case for context click at misspelled word boundary

On most platforms, right-clicking a misspelled word only selects it and shows
suggestions in the context menu if you right-clik in the interior of the word.
In https://codereview.chromium.org/2925363002, I modified some of the spellcheck
code (specifically, the method that replaces the misspelled word) to support the
case where we have a caret selection immediately before or after a word, since
tapping at a word boundary should open the spellcheck menu on Android (this
feature is still in progress, but this is the behavior for a native EditText
widget, so we're trying to match it.)

This CL adds a test case to verify we don't accidentally change the behavior for
other platforms.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2932123002
Cr-Commit-Position: refs/heads/master@{#478805}

[modify] https://crrev.com/e7fe96812663e16778c2517bd3587eded541088e/third_party/WebKit/LayoutTests/editing/spelling/context_click_select_misspelling.html

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afd1411d6b0dbb13fc94314c745755fd76bd763b

commit afd1411d6b0dbb13fc94314c745755fd76bd763b
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 16 21:07:05 2017

Add DocumentShutdown{Notifier,Observer}

This is a subclass of LifecycleNotifier that classes can use to be notified when
the Document they're attached to is shut down.
SynchronousMutation{Notifier,Observer} provides the same functionality but is
more performance sensitive because it also gets notified when
Node::appendChild() and Node::removeChild() are called.

The idea to add this came from the code review of this CL:
https://codereview.chromium.org/2931443003#msg72

BUG= 715365 

Review-Url: https://codereview.chromium.org/2939223003
Cr-Commit-Position: refs/heads/master@{#480164}

[modify] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/BUILD.gn
[modify] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/Document.h
[add] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/DocumentShutdownNotifier.cpp
[add] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/DocumentShutdownNotifier.h
[add] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/DocumentShutdownObserver.cpp
[add] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/DocumentShutdownObserver.h
[modify] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/afd1411d6b0dbb13fc94314c745755fd76bd763b/third_party/WebKit/Source/core/dom/SynchronousMutationObserver.h

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f55a0f50100a091e09af261e0350f0efae18d872

commit f55a0f50100a091e09af261e0350f0efae18d872
Author: rlanday <rlanday@chromium.org>
Date: Fri Jun 16 21:20:40 2017

Remove zero-length spaces from Android spelling suggestions

It turns out that the Android spellchecker is adding zero-length spaces (\u200b)
to the end of some suggestions (it might be particular to multi-word suggestions
but I'm not sure). We need to filter these out because applying these suggestions
(support being added in https://codereview.chromium.org/2931443003) is causing
composition underlines to exhibit odd behavior.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2943033002
Cr-Commit-Position: refs/heads/master@{#480173}

[modify] https://crrev.com/f55a0f50100a091e09af261e0350f0efae18d872/components/spellcheck/browser/android/java/src/org/chromium/components/spellcheck/SpellCheckerSessionBridge.java

Project Member

Comment 18 by bugdroid1@chromium.org, Jun 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c0e31689b5b37b8f128db8b8475fa7744e675d47

commit c0e31689b5b37b8f128db8b8475fa7744e675d47
Author: rlanday <rlanday@chromium.org>
Date: Wed Jun 21 02:17:18 2017

Add LayoutTheme::PlatformActiveSpellingMarkerHighlightColor()

I'm working on adding support for the Android spellcheck menu in
https://codereview.chromium.org/2931443003

and we need to add another color to LayoutTheme to represent what color we
should highlight a misspelled word while the menu is active. Android uses the
same red color as the spellcheck underline but with some transparency added. We
decided that it's better to add a separate method to LayoutTheme for getting the
color than to assume that we'll want to do an alpha blend on every platform that
might ever support this menu.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2953493002
Cr-Commit-Position: refs/heads/master@{#481085}

[modify] https://crrev.com/c0e31689b5b37b8f128db8b8475fa7744e675d47/third_party/WebKit/Source/core/layout/LayoutTheme.cpp
[modify] https://crrev.com/c0e31689b5b37b8f128db8b8475fa7744e675d47/third_party/WebKit/Source/core/layout/LayoutTheme.h

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c17fd58d40e84744ce40192a457888de8334a54

commit 6c17fd58d40e84744ce40192a457888de8334a54
Author: rlanday <rlanday@chromium.org>
Date: Thu Jun 22 17:43:45 2017

Add DocumentMarkerList::MarkersIntersectingRange()

Currently, the only way to find the list of markers intersecting a range is to
iterate through the nodes the range contains, and get the complete list of nodes
for each range from DocumentMarkerController, and do a linear search through the
list. This is inefficient if we have a text node with a lot of markers, since we
could be doing a binary search instead of a linear scan.

This CL prepares us to add a
DocumentMarkerController::MarkersIntersectingRange() method by supporting the
necessary operation on the DocumentMarkerLists for each type (this is necessary
because DocumentMarkerController shouldn't be making any assumptions about how
each list is stored).

BUG= 715365 

Review-Url: https://codereview.chromium.org/2952953002
Cr-Commit-Position: refs/heads/master@{#481588}

[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImpl.cpp
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/ActiveSuggestionMarkerListImpl.h
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.cpp
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/CompositionMarkerListImpl.h
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/DocumentMarkerList.h
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.cpp
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditor.h
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/DocumentMarkerListEditorTest.cpp
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.cpp
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/SpellCheckMarkerListImpl.h
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.cpp
[modify] https://crrev.com/6c17fd58d40e84744ce40192a457888de8334a54/third_party/WebKit/Source/core/editing/markers/TextMatchMarkerListImpl.h

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d93c38b3aecacd999960ae3a037e0dd7b67f9a3

commit 8d93c38b3aecacd999960ae3a037e0dd7b67f9a3
Author: Ryan Landay <rlanday@chromium.org>
Date: Tue Jul 11 01:30:23 2017

Set android:colorTheme in Chrome for Android's MainTheme

I am working on re-implementing the Android text edit widget's spell check menu
in Chrome and the Android WebView. The buttons in the native Android spell check
menu use the value of android:colorAccent set by the embedding app. To make the
WebView spell check menu have the same behavior, we need to also make it use
android:colorAccent. But to make the menu use the correct (according to
hannahs@, our UX designer) blue color, and not the default green color, we need
to set colorAccent in our MainTheme. This change does that.

I'm assuming that we don't currently have anything that is relying on this being
the existing green color (since that wouldn't match the color our designer said
we're supposed to use for these UI elements anyway). We do currently have some
themes (e.g. FullscreenWhite and PreferencesDialogTheme) that are already
setting this color to other values for specific parts of the app though.

Note: I've tested this change on Jelly Bean and the app seems to run OK, despite
android:colorAccent having been added in a later API version.

Bug:  715365 
Change-Id: I2690c014d18211304ace6894f5a2cea0db72a1af
Reviewed-on: https://chromium-review.googlesource.com/566031
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485486}
[modify] https://crrev.com/8d93c38b3aecacd999960ae3a037e0dd7b67f9a3/chrome/android/java/res/values-v17/styles.xml

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2938cdc186f145300fab82e22111aefc656e39a0

commit 2938cdc186f145300fab82e22111aefc656e39a0
Author: Ryan Landay <rlanday@chromium.org>
Date: Wed Jul 12 22:53:27 2017

Add UiUtils.computeMaxWidthOfListAdapterItems()

I am factoring out some code in DropdownPopupWindow.measureContentWidth() that
determines the widest view in a ListView into a helper method in UiUtils so I
can use it for the spell check menu I'm working on in
https://codereview.chromium.org/2931443003.

Bug:  715365 
Change-Id: I59bfc348eac62d07c6deb375bca6b808856911dd
Reviewed-on: https://chromium-review.googlesource.com/566594
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486145}
[modify] https://crrev.com/2938cdc186f145300fab82e22111aefc656e39a0/ui/android/java/src/org/chromium/ui/DropdownPopupWindow.java
[modify] https://crrev.com/2938cdc186f145300fab82e22111aefc656e39a0/ui/android/java/src/org/chromium/ui/UiUtils.java

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56

commit e4a6ec83ec32b4f139794f39bf2cbb0b25617b56
Author: rlanday <rlanday@chromium.org>
Date: Wed Jul 26 01:03:53 2017

Add support for Android spellcheck menu in Chrome/WebViews

Currently, in Chrome and in WebViews, tapping misspelled words on Android (that
have red underlines) doesn't do anything. In a native text box, tapping a
misspelled word opens a menu that allows the user to pick from a list of
suggested replacements (if any), delete the misspelled word, or add it to the
system dictionary. This CL ports this menu to Chrome and WebViews.

Before this project, the list of spelling suggestions from the Android
spellchecker was being discarded. https://codereview.chromium.org/2905003003
fixes this so the suggestions are passed through to the Spelling
DocumentMarkers. This CL adds code to SelectionController::HandleSingleClick()
to check if the user tapped on a misspelled word, and if so, send a Mojo
message from the renderer to the browser to start a timer to open the
spellcheck menu unless we receive another tap within the double-tap timeout
threshold (note: this CL restricts the spellcheck menu to Android by not
handling this message on any other platform). If the timer expires, a message
is sent back to the renderer to try to open the spellcheck menu (nothing
happens if the spelling marker no longer exists). If the renderer decides the
spellcheck menu should be opened, it highlights the misspelled word with in red
using an ActiveSuggestionMarker (added in
https://codereview.chromium.org/2923033002), hides the caret, and sends another
message back to the browser with a list of spelling suggestions (if any).

The layout files for the menu and the logic for showing it were ported over from
the implementation for the same menu in android.widget.Editor. Some of the XML
files were modified slightly to avoid dependencies on internal Android system
resources or newer Android versions.

Tapping outside the spellcheck menu closes it, and sends a message to Blink
telling it to remove the highlight and show the caret again. There are three
other messages for applying a suggestion, deleting the misspelled word, and
adding the word to the dictionary. For adding the word to the dictionary, I'm
using the same Android intent the native textbox uses, with the same wonky
behavior that the spellcheck marker is removed immediately when the user taps
"Add to dictionary", before the user has gone through the confirmation dialog.

BUG= 715365 

Review-Url: https://codereview.chromium.org/2931443003
Cr-Commit-Position: refs/heads/master@{#489505}

[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/android_webview/browser/aw_browser_manifest_overlay.json
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/chrome/android/java_sources.gni
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/chrome/android/javatests/src/org/chromium/chrome/browser/input/TextSuggestionMenuTest.java
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/BUILD.gn
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/DEPS
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/android/browser_jni_registrar.cc
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/android/text_suggestion_host_android.cc
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/android/text_suggestion_host_android.h
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/android/text_suggestion_host_mojo_impl_android.cc
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/android/text_suggestion_host_mojo_impl_android.h
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/BUILD.gn
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/drawable/floating_popup_background_light.xml
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/layout/text_edit_suggestion_container.xml
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/layout/text_edit_suggestion_item.xml
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/layout/text_edit_suggestion_list_footer.xml
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/values-v21/styles.xml
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/values/colors.xml
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/res/values/dimens.xml
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/src/org/chromium/content/browser/input/SuggestionsPopupWindow.java
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/src/org/chromium/content/browser/input/TextSuggestionHost.java
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/java/strings/android_content_strings.grd
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/content/public/app/mojo/content_renderer_manifest.json
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/SelectionController.cpp
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionBackendImpl.cpp
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionBackendImpl.h
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.h
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionControllerTest.cpp
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/Source/modules/ModulesInitializer.cpp
[modify] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/public/platform/input_host.mojom
[add] https://crrev.com/e4a6ec83ec32b4f139794f39bf2cbb0b25617b56/third_party/WebKit/public/platform/input_messages.mojom

Status: Fixed (was: Started)
The code for this has been landed so this feature should be available in M62.

Sign in to add a comment