Reland "Make assistive window show with autocorrect manager"

Call setAssistiveWindowProperties only when needed.

This stops breaking ChromeVox




This reverts commit a7f6e1a1.

Reason for revert: Adding a fix so that ChromeVox editing notifications still work

Original change's description:
> Revert "Make assistive window show with autocorrect manager"
>
> Fixed: 1146877
> TBR=jopalmer@chromium.org
>
> Reason for revert:
> Breaks ChromeVox; found via bisect; see bug.
>
> This reverts commit 6cbffa8a.
>
> Dependent changes (also reverted):
> 73bcb673
> 84c821b5
> b197149c
>
> Change-Id: I65bc46c273cd64555676073cf6cd172128cfe068
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2529695
> Commit-Queue: David Tseng <dtseng@chromium.org>
> Reviewed-by: David Tseng <dtseng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825948}



Change-Id: I3d4b08c6197f287dbf55fca2d377ce577c81a617
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530953Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: John Palmer <jopalmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826690}

chrome/app/chromeos_strings.grdp

@@ -5845,6 +5845,15 @@ You can manage this account’s settings by installing the Family Link app on yo
   <message name="IDS_SUGGESTION_INSERTED" desc="String announced by screenreader when a suggestion is inserted.">
     Suggestion inserted
   </message>
+  <message name="IDS_SUGGESTION_AUTOCORRECT_UNDONE" desc="String announced by screenreader when an autocorrected word is reverted. Example, user types 'hllo', system autocorrects to 'hello' but then that autocorrection is manually undone by the user.">
+    correction undone
+  </message>
+  <message name="IDS_SUGGESTION_AUTOCORRECT_UNDO_BUTTON" desc="String announced by screenreader when a button to undo an autocorrect is highlighted. Example, user types 'hllo', system autocorrects to 'hello', user then prepares to revert the autocorrect.">
+    Undo autocorrect button. Revert to <ph name="TYPED_WORD">$1<ex>hllo</ex></ph>.  Press enter to activate, escape to dismiss.
+  </message>
+  <message name="IDS_SUGGESTION_AUTOCORRECT_UNDO_WINDOW_SHOWN" desc="String announced by screenreader when a button to undo an autocorrect is shown. Example, user types 'hllo', system autocorrects to 'hello', user then prepares to revert the autocorrect.">
+    Autocorrect undo dialog is shown for <ph name="TYPED_WORD">$1<ex>hllo</ex></ph> corrected to <ph name="CORRECTED_WORD">$2<ex>hello</ex></ph>.  Press up arrow to access, escape to ignore.
+  </message>
 
   <!-- Strings for Custom Tab UI -->
   <message name="IDS_CUSTOM_TABS_ACTION_MENU_ACCESSIBLE_NAME" desc="Accessible name for the overflow menu of the Chrome Custom Tab toolbar.">

chrome/app/chromeos_strings_grdp/IDS_SUGGESTION_AUTOCORRECT_UNDONE.png.sha1

+82ea2c6dd7ddba8fa57359482cdad1304999a830
\ No newline at end of file

chrome/app/chromeos_strings_grdp/IDS_SUGGESTION_AUTOCORRECT_UNDO_BUTTON.png.sha1

+82ea2c6dd7ddba8fa57359482cdad1304999a830
\ No newline at end of file

chrome/app/chromeos_strings_grdp/IDS_SUGGESTION_AUTOCORRECT_UNDO_WINDOW_SHOWN.png.sha1

+82ea2c6dd7ddba8fa57359482cdad1304999a830
\ No newline at end of file

chrome/browser/chromeos/input_method/autocorrect_manager.cc

@@ -4,51 +4,147 @@
 
 #include "chrome/browser/chromeos/input_method/autocorrect_manager.h"
 
+#include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chromeos/input_method/assistive_window_properties.h"
+#include "chrome/grit/generated_resources.h"
+#include "ui/base/ime/chromeos/ime_bridge.h"
+#include "ui/base/ime/chromeos/ime_input_context_handler_interface.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace chromeos {
 
-const int kKeysUntilAutocorrectWindowHides = 4;
+constexpr int kKeysUntilAutocorrectWindowHides = 4;
 
-AutocorrectManager::AutocorrectManager(InputMethodEngineBase* engine)
+AutocorrectManager::AutocorrectManager(InputMethodEngine* engine)
     : engine_(engine) {}
 
 void AutocorrectManager::MarkAutocorrectRange(const std::string& corrected_word,
+                                              const std::string& typed_word,
                                               int start_index) {
   // TODO(crbug/1111135): call setAutocorrectTime() (for metrics)
   // TODO(crbug/1111135): record metric (coverage)
+  last_typed_word_ = typed_word;
+  last_corrected_word_ = corrected_word;
   key_presses_until_underline_hide_ = kKeysUntilAutocorrectWindowHides;
   ClearUnderline();
+
   if (context_id_ != -1) {
-    std::string error;
-    // TODO(crbug/1111135): error handling
-    engine_->SetAutocorrectRange(context_id_, base::UTF8ToUTF16(corrected_word),
-                                 start_index, start_index, &error);
+    engine_->SetAutocorrectRange(base::UTF8ToUTF16(corrected_word), start_index,
+                                 start_index + corrected_word.length());
   }
 }
 
-void AutocorrectManager::OnKeyEvent() {
-  if (key_presses_until_underline_hide_ < 0) {
-    return;
+bool AutocorrectManager::OnKeyEvent(
+    const InputMethodEngineBase::KeyboardEvent& event) {
+  if (event.type != "keydown") {
+    return false;
+  }
+  if (event.key == "Up" && window_visible) {
+    std::string error;
+    auto button = ui::ime::AssistiveWindowButton();
+    button.id = ui::ime::ButtonId::kUndo;
+    button.window_type = ui::ime::AssistiveWindowType::kUndoWindow;
+    button.announce_string =
+        l10n_util::GetStringFUTF8(IDS_SUGGESTION_AUTOCORRECT_UNDO_BUTTON,
+                                  base::UTF8ToUTF16(last_typed_word_));
+    engine_->SetButtonHighlighted(context_id_, button, true, &error);
+    button_highlighted = true;
+    return true;
   }
-  --key_presses_until_underline_hide_;
-  if (key_presses_until_underline_hide_ < 1) {
+  if (event.key == "Enter" && window_visible && button_highlighted) {
+    UndoAutocorrect();
+    return true;
+  }
+  if (key_presses_until_underline_hide_ > 0) {
+    --key_presses_until_underline_hide_;
+  }
+  if (key_presses_until_underline_hide_ == 0) {
     ClearUnderline();
   }
+  return false;
 }
 
 void AutocorrectManager::ClearUnderline() {
-  // TODO(crbug/1111135): error handling
+  engine_->SetAutocorrectRange(/*autocorrect text=*/base::string16(),
+                               /*start=*/0,
+                               /*end=*/std::numeric_limits<uint32_t>::max());
   // TODO(b/171924347): expose engine->clearAutocorrectRange() and use it here.
+}
+
+void AutocorrectManager::OnSurroundingTextChanged(const base::string16& text,
+                                                  const int cursor_pos,
+                                                  const int anchpr_pos) {
   std::string error;
-  engine_->SetAutocorrectRange(
-      context_id_,
-      /*autocorrect_text=*/base::string16(),
-      /*start=*/0, /*end=*/std::numeric_limits<uint32_t>::max(), &error);
+  const gfx::Range range = engine_->GetAutocorrectRange();
+  if (!range.is_empty() && cursor_pos >= range.start() &&
+      cursor_pos <= range.end()) {
+    if (!window_visible) {
+      chromeos::AssistiveWindowProperties properties;
+      properties.type = ui::ime::AssistiveWindowType::kUndoWindow;
+      properties.visible = true;
+      properties.announce_string = l10n_util::GetStringFUTF8(
+          IDS_SUGGESTION_AUTOCORRECT_UNDO_WINDOW_SHOWN,
+          base::UTF8ToUTF16(last_typed_word_),
+          base::UTF8ToUTF16(last_corrected_word_));
+      window_visible = true;
+      button_highlighted = false;
+      engine_->SetAssistiveWindowProperties(context_id_, properties, &error);
+    }
+    key_presses_until_underline_hide_ = kKeysUntilAutocorrectWindowHides;
+  } else if (window_visible) {
+    chromeos::AssistiveWindowProperties properties;
+    properties.type = ui::ime::AssistiveWindowType::kUndoWindow;
+    properties.visible = false;
+    window_visible = false;
+    button_highlighted = false;
+    engine_->SetAssistiveWindowProperties(context_id_, properties, &error);
+  }
 }
 
 void AutocorrectManager::OnFocus(int context_id) {
   context_id_ = context_id;
 }
 
+void AutocorrectManager::UndoAutocorrect() {
+  // TODO(crbug/1111135): error handling and metrics
+  std::string error;
+  chromeos::AssistiveWindowProperties properties;
+  properties.type = ui::ime::AssistiveWindowType::kUndoWindow;
+  properties.visible = false;
+  window_visible = false;
+  button_highlighted = false;
+  window_visible = false;
+  engine_->SetAssistiveWindowProperties(context_id_, properties, &error);
+
+  const gfx::Range range = engine_->GetAutocorrectRange();
+  const ui::SurroundingTextInfo surrounding_text =
+      ui::IMEBridge::Get()->GetInputContextHandler()->GetSurroundingTextInfo();
+  // TODO(crbug/1111135): Can we get away with deleting less text?
+  // This will not quite work properly if there is text actually highlighted,
+  // and cursor is at end of the highlight block, but no easy way around it.
+  // First delete everything before cursor.
+  engine_->DeleteSurroundingText(
+      context_id_, -static_cast<int>(surrounding_text.selection_range.start()),
+      surrounding_text.surrounding_text.length(), &error);
+  // Submit the text after the cursor in composition mode to leave the cursor at
+  // the start
+  engine_->SetComposition(
+      context_id_,
+      base::UTF16ToUTF8(surrounding_text.surrounding_text.substr(range.end()))
+          .c_str(),
+      /*selection_start=*/0, /*selection_end=*/0, /*cursor=*/0, /*segments=*/{},
+      &error);
+  engine_->FinishComposingText(context_id_, &error);
+  // Insert the text before the cursor - now there should be the correct text
+  // and the cursor position will not have changed.
+  engine_->CommitText(
+      context_id_,
+      (base::UTF16ToUTF8(
+           surrounding_text.surrounding_text.substr(0, range.start())) +
+       last_typed_word_)
+          .c_str(),
+      &error);
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/input_method/autocorrect_manager.h

@@ -7,12 +7,9 @@
 
 #include <string>
 
-#include "chrome/browser/chromeos/input_method/emoji_suggester.h"
+#include "chrome/browser/chromeos/input_method/assistive_window_controller.h"
 #include "chrome/browser/chromeos/input_method/input_method_engine.h"
 #include "chrome/browser/chromeos/input_method/input_method_engine_base.h"
-#include "chrome/browser/chromeos/input_method/personal_info_suggester.h"
-#include "chrome/browser/chromeos/input_method/suggester.h"
-#include "chrome/browser/chromeos/input_method/suggestion_enums.h"
 
 namespace chromeos {
 
@@ -24,7 +21,7 @@ class AutocorrectManager {
  public:
   // Engine is used to interact with the text field, and is assumed to be
   // valid for the entire lifetime of the autocorrect manager.
-  explicit AutocorrectManager(InputMethodEngineBase* engine);
+  explicit AutocorrectManager(InputMethodEngine* engine);
 
   AutocorrectManager(const AutocorrectManager&) = delete;
   AutocorrectManager& operator=(const AutocorrectManager&) = delete;
@@ -32,19 +29,31 @@ class AutocorrectManager {
   // Called by input method engine on autocorrect to initially show underline.
   // Needs to be called after the autocorrected text (corrected_word, offset by
   // start_index code points in SurroundingInfo) has been committed.
-  void MarkAutocorrectRange(const std::string& corrected_word, int start_index);
+  void MarkAutocorrectRange(const std::string& corrected_word,
+                            const std::string& typed_word,
+                            int start_index);
   // To hide the underline after enough keypresses, this class intercepts
-  // keystrokes.
-  void OnKeyEvent();
+  // keystrokes. Returns whether the keypress has now been handled.
+  bool OnKeyEvent(const InputMethodEngineBase::KeyboardEvent& event);
   // Indicates a new text field is focused, used to save context ID.
   void OnFocus(int context_id);
+  // To show the undo window when cursor is in an autocorrected word, this class
+  // is notified of surrounding text changes.
+  void OnSurroundingTextChanged(const base::string16& text,
+                                int cursor_pos,
+                                int anchor_pos);
+  void UndoAutocorrect();
 
  private:
   void ClearUnderline();
 
   int key_presses_until_underline_hide_ = 0;
   int context_id_ = -1;
-  InputMethodEngineBase* const engine_;
+  InputMethodEngine* const engine_;
+  std::string last_typed_word_;
+  std::string last_corrected_word_;
+  bool window_visible = false;
+  bool button_highlighted = false;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/input_method/input_method_engine.h

@@ -185,6 +185,8 @@ class InputMethodEngine : public InputMethodEngineBase,
                            uint32_t start,
                            uint32_t end) override;
 
+  gfx::Range GetAutocorrectRange() override;
+
  private:
   // InputMethodEngineBase:
   void UpdateComposition(const ui::CompositionText& composition_text,
@@ -199,7 +201,6 @@ class InputMethodEngine : public InputMethodEngineBase,
       uint32_t end,
       const std::vector<ui::ImeTextSpan>& text_spans) override;
 
-  gfx::Range GetAutocorrectRange() override;
 
   gfx::Rect GetAutocorrectCharacterBounds() override;
 

chrome/browser/chromeos/input_method/native_input_method_engine.cc

@@ -155,7 +155,8 @@ bool NativeInputMethodEngine::IsConnectedForTesting() const {
 void NativeInputMethodEngine::OnAutocorrect(std::string typed_word,
                                             std::string corrected_word,
                                             int start_index) {
-  autocorrect_manager_->MarkAutocorrectRange(corrected_word, start_index);
+  autocorrect_manager_->MarkAutocorrectRange(corrected_word, typed_word,
+                                             start_index);
 }
 
 NativeInputMethodEngine::ImeObserver*
@@ -257,7 +258,10 @@ void NativeInputMethodEngine::ImeObserver::OnKeyEvent(
       return;
     }
   }
-  autocorrect_manager_->OnKeyEvent();
+  if (autocorrect_manager_->OnKeyEvent(event)) {
+    std::move(callback).Run(true);
+    return;
+  }
   auto key_event = ime::mojom::PhysicalKeyEvent::New(
       event.type == "keydown" ? ime::mojom::KeyEventType::kKeyDown
                               : ime::mojom::KeyEventType::kKeyUp,
@@ -314,6 +318,7 @@ void NativeInputMethodEngine::ImeObserver::OnSurroundingTextChanged(
     assistive_suggester_->OnSurroundingTextChanged(text, cursor_pos,
                                                    anchor_pos);
   }
+  autocorrect_manager_->OnSurroundingTextChanged(text, cursor_pos, anchor_pos);
   if (ShouldUseFstMojoEngine(engine_id) && remote_to_engine_.is_bound()) {
     auto selection = ime::mojom::SelectionRange::New();
     selection->anchor = anchor_pos;
@@ -366,6 +371,8 @@ void NativeInputMethodEngine::ImeObserver::OnAssistiveWindowButtonClicked(
       }
       break;
     case ui::ime::ButtonId::kUndo:
+      autocorrect_manager_->UndoAutocorrect();
+      break;
     case ui::ime::ButtonId::kAddToDictionary:
     case ui::ime::ButtonId::kNone:
       base_observer_->OnAssistiveWindowButtonClicked(button);

chrome/browser/chromeos/input_method/native_input_method_engine.h

@@ -47,6 +47,10 @@ class NativeInputMethodEngine : public InputMethodEngine {
     return assistive_suggester_;
   }
 
+  AutocorrectManager* get_autocorrect_manager_for_testing() {
+    return autocorrect_manager_;
+  }
+
   // Used to show special UI to user for interacting with autocorrected text.
   void OnAutocorrect(std::string typed_word,
                      std::string corrected_word,

chrome/browser/chromeos/input_method/native_input_method_engine_browsertest.cc

@@ -12,6 +12,7 @@
 #include "base/test/task_environment.h"
 #include "base/values.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
+#include "chrome/browser/chromeos/input_method/assistive_window_controller.h"
 #include "chrome/browser/chromeos/input_method/suggestion_enums.h"
 #include "chrome/browser/chromeos/input_method/textinput_test_helper.h"
 #include "chrome/browser/profiles/profile.h"
@@ -214,6 +215,13 @@ class NativeInputMethodEngineTest : public InProcessBrowserTest,
     waiterReleased.Wait();
   }
 
+  void DispatchKeyPresses(const std::vector<ui::KeyboardCode>& codes,
+                          bool need_flush) {
+    for (const ui::KeyboardCode& code : codes) {
+      DispatchKeyPress(code, need_flush);
+    }
+  }
+
   void SetFocus(ui::TextInputClient* client) {
     input_method_.SetFocusedTextInputClient(client);
   }
@@ -637,6 +645,148 @@ IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, DestroyProfile) {
   EXPECT_EQ(engine_.GetPrefChangeRegistrarForTesting(), nullptr);
 }
 
+IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest,
+                       HighlightsOnAutocorrectThenDismissesHighlight) {
+  engine_.Enable(kEngineIdUs);
+  ui::DummyTextInputClient text_input_client(ui::TEXT_INPUT_TYPE_TEXT);
+  SetFocus(&text_input_client);
+  // Input the corrected word.
+  DispatchKeyPresses(
+      {
+          ui::VKEY_C,
+          ui::VKEY_O,
+          ui::VKEY_R,
+          ui::VKEY_R,
+          ui::VKEY_E,
+          ui::VKEY_C,
+          ui::VKEY_T,
+          ui::VKEY_E,
+          ui::VKEY_D,
+      },
+      false);
+
+  engine_.OnAutocorrect("typed", "corrected", 0);
+
+  EXPECT_FALSE(engine_.GetAutocorrectRange().is_empty());
+
+  DispatchKeyPress(ui::KeyboardCode::VKEY_A, false);
+  DispatchKeyPress(ui::KeyboardCode::VKEY_A, false);
+  DispatchKeyPress(ui::KeyboardCode::VKEY_A, false);
+
+  // Highlighting should only go away after 4 keypresses.
+  EXPECT_FALSE(engine_.GetAutocorrectRange().is_empty());
+
+  DispatchKeyPress(ui::KeyboardCode::VKEY_A, false);
+
+  EXPECT_TRUE(engine_.GetAutocorrectRange().is_empty());
+
+  SetFocus(nullptr);
+}
+
+IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest,
+                       ShowsAndHidesAutocorrectUndoWindow) {
+  engine_.Enable(kEngineIdUs);
+  chromeos::TextInputTestHelper helper(GetBrowserInputMethod());
+  SetUpTextInput(helper);
+  const base::string16 prefix_text = base::UTF8ToUTF16("corrected ");
+  helper.GetTextInputClient()->InsertText(prefix_text);
+  helper.WaitForSurroundingTextChanged(prefix_text);
+
+  engine_.OnAutocorrect("typed", "corrected", 0);
+
+  auto* controller =
+      ((chromeos::input_method::
+            AssistiveWindowController*)(ui::IMEBridge::Get()
+                                            ->GetAssistiveWindowHandler()));
+
+  EXPECT_FALSE(controller->GetUndoWindowForTesting());
+
+  // Move cursor back into the autocorrected word to show the window.
+  helper.GetTextInputClient()->ExtendSelectionAndDelete(1, 0);
+  helper.WaitForSurroundingTextChanged(base::UTF8ToUTF16("corrected"));
+
+  EXPECT_TRUE(controller->GetUndoWindowForTesting());
+  EXPECT_TRUE(controller->GetUndoWindowForTesting()->GetVisible());
+
+  SetFocus(nullptr);
+}
+
+IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest, RevertsAutocorrect) {
+  engine_.Enable(kEngineIdUs);
+  chromeos::TextInputTestHelper helper(GetBrowserInputMethod());
+  SetUpTextInput(helper);
+  const base::string16 corrected_text =
+      base::UTF8ToUTF16("hello corrected world");
+  const base::string16 typed_text = base::UTF8ToUTF16("hello typed world");
+  helper.GetTextInputClient()->InsertText(corrected_text);
+  helper.WaitForSurroundingTextChanged(corrected_text);
+  EXPECT_EQ(ui::IMEBridge::Get()
+                ->GetInputContextHandler()
+                ->GetSurroundingTextInfo()
+                .surrounding_text,
+            corrected_text);
+
+  engine_.OnAutocorrect("typed", "corrected", 6);
+
+  // Move cursor into the corrected word, sending VKEY_LEFT fails, so use JS.
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(content::ExecuteScript(
+      tab, "document.getElementById('text_id').setSelectionRange(8,8)"));
+
+  helper.WaitForSurroundingTextChanged(corrected_text, gfx::Range(8, 8));
+
+  engine_.get_autocorrect_manager_for_testing()->UndoAutocorrect();
+
+  helper.WaitForSurroundingTextChanged(typed_text);
+
+  EXPECT_EQ(ui::IMEBridge::Get()
+                ->GetInputContextHandler()
+                ->GetSurroundingTextInfo()
+                .surrounding_text,
+            typed_text);
+
+  SetFocus(nullptr);
+}
+
+IN_PROC_BROWSER_TEST_F(NativeInputMethodEngineTest,
+                       RevertsAutocorrectWithKeyboard) {
+  engine_.Enable(kEngineIdUs);
+
+  chromeos::TextInputTestHelper helper(GetBrowserInputMethod());
+  SetUpTextInput(helper);
+  const base::string16 corrected_text = base::UTF8ToUTF16("corrected");
+  const base::string16 typed_text = base::UTF8ToUTF16("typed");
+  helper.GetTextInputClient()->InsertText(corrected_text);
+  helper.WaitForSurroundingTextChanged(corrected_text);
+  EXPECT_EQ(ui::IMEBridge::Get()
+                ->GetInputContextHandler()
+                ->GetSurroundingTextInfo()
+                .surrounding_text,
+            corrected_text);
+
+  engine_.OnAutocorrect("typed", "corrected", 0);
+  // Move cursor into the corrected word, sending VKEY_LEFT fails, so use JS.
+  content::WebContents* tab =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(content::ExecuteScript(
+      tab, "document.getElementById('text_id').setSelectionRange(2,2)"));
+  helper.WaitForSurroundingTextChanged(corrected_text, gfx::Range(2, 2));
+
+  DispatchKeyPress(ui::VKEY_UP, false);
+  DispatchKeyPress(ui::VKEY_RETURN, false);
+
+  helper.WaitForSurroundingTextChanged(typed_text);
+
+  EXPECT_EQ(ui::IMEBridge::Get()
+                ->GetInputContextHandler()
+                ->GetSurroundingTextInfo()
+                .surrounding_text,
+            typed_text);
+
+  SetFocus(nullptr);
+}
+
 class NativeInputMethodEngineAssistiveOff : public InProcessBrowserTest {
  public:
   NativeInputMethodEngineAssistiveOff() {