MacViews/a11y: Allow accessibility clients to update the selected text.

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 
 #include <set>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 401 matching lines @@
   else
     model()->OnUpOrDownKeyPressed(event.IsShiftDown() ? -1 : 1);
 
   return true;
 }
 
 void OmniboxViewViews::AccessibilitySetValue(const base::string16& new_value) {
   SetUserText(new_value, true);
 }
 
+void OmniboxViewViews::AccessibilityReplaceSelection(
+    const base::string16& new_value) {
+  // This should work even when focus is not on the omnibox, so restore the
+  // selection saved separately in OnBlur().
+  if (!HasFocus() && saved_selection_for_focus_change_.IsValid()) {
+    SelectRange(saved_selection_for_focus_change_);

[tapted, 2016/09/27 07:16:30]

I don't think we need to select it (I'm actually not sure what it means to
select when there is no focus), but I think we can just apply the selection like

gfx::Range selection_range = saved_selection_for_focus_change_.IsValid()
    ? saved_selection_for_focus_change_
    : GetSelectedRange();

[Patti Lor, 2016/10/20 04:19:34]

Done.

+    saved_selection_for_focus_change_ = gfx::Range::InvalidRange();
+  }
+  gfx::Range selection_range = GetSelectedRange();
+  base::string16 current_text = GetText();
+  base::string16 new_string =
+      current_text.substr(0, selection_range.GetMin()) + new_value +
+      current_text.substr(selection_range.GetMax(), current_text.length());

[tapted, 2016/09/27 07:16:30]

`current_text.length()` shouldn't be needed on the third component

[Patti Lor, 2016/10/20 04:19:34]

Done.

+  SetUserText(new_string, true);

[tapted, 2016/09/27 07:16:30]

Calling AccessibilitySetValue(new_string) is probably a slight improvement

[Patti Lor, 2016/10/20 04:19:34]

No longer needed after your |replace| arg suggestion :)

+  SetSelectionRange(gfx::Range(selection_range.GetMin() + new_value.length()));

[tapted, 2016/09/27 07:16:30]

Same here - I don't think we can set a selection without focus. Perhaps

selection_range.set_end(
    selection_range.end() - selection_range.length() + new_value.length());
if (HasFocus())
  SetSelectionRange(selection_range)
else
  saved_selection_for_focus_change_ = selection_range;

[Patti Lor, 2016/10/20 04:19:34]

Sorry - this is an inconsistency that I've now deleted. As per the other
comment, Cocoa doesn't actually re-apply the selection after replacing it (the
behaviour is the same as you had typed something while having selected text
focused).

+}
+
 void OmniboxViewViews::UpdateSecurityLevel() {
   security_level_ = controller()->GetToolbarModel()->GetSecurityLevel(false);
 }
 
 void OmniboxViewViews::SetWindowTextAndCaretPos(const base::string16& text,
                                                 size_t caret_pos,
                                                 bool update_popup,
                                                 bool notify_text_changed) {
   const gfx::Range range(caret_pos, caret_pos);
   SetTextAndSelectedRange(text, range);
@@ skipping 302 matching lines @@
   return Textfield::SkipDefaultKeyEventProcessing(event);
 }
 
 void OmniboxViewViews::GetAccessibleState(ui::AXViewState* state) {
   state->role = ui::AX_ROLE_TEXT_FIELD;
   state->name = l10n_util::GetStringUTF16(IDS_ACCNAME_LOCATION);
   state->value = GetText();
 
   base::string16::size_type entry_start;
   base::string16::size_type entry_end;
   GetSelectionBounds(&entry_start, &entry_end);

[tapted, 2016/09/27 07:16:30]

move to an `else`?

[Patti Lor, 2016/10/20 04:19:33]

Done.

+  // Selection information is saved separately when focus is moved off the
+  // current window - use that when there is no focus and it's valid.
+  if (!HasFocus() && saved_selection_for_focus_change_.IsValid()) {

[tapted, 2016/09/27 07:16:30]

`!HasFocus()` might not be required -- pretty sure
`saved_selection_for_focus_change_.IsValid()` \implies{} !HasFocus()

[Patti Lor, 2016/10/20 04:19:34]

Done.

+    entry_start = saved_selection_for_focus_change_.start();
+    entry_end = saved_selection_for_focus_change_.end();
+  }
   state->selection_start = entry_start;
   state->selection_end = entry_end;
 
   if (popup_window_mode_) {
     state->AddStateFlag(ui::AX_STATE_READ_ONLY);
   } else {
     state->AddStateFlag(ui::AX_STATE_EDITABLE);
     state->set_value_callback =
         base::Bind(&OmniboxViewViews::AccessibilitySetValue,
                    weak_ptr_factory_.GetWeakPtr());
+    state->replace_selection_callback =
+        base::Bind(&OmniboxViewViews::AccessibilityReplaceSelection,
+                   weak_ptr_factory_.GetWeakPtr());
   }
 }
 
 void OmniboxViewViews::OnFocus() {
   views::Textfield::OnFocus();
   // TODO(oshima): Get control key state.
   model()->OnSetFocus(false);
   // Don't call controller()->OnSetFocus, this view has already acquired focus.
 
   // Restore the selection we saved in OnBlur() if it's still valid.
@@ skipping 311 matching lines @@
       paste_position + 1, IDS_PASTE_AND_GO, IDS_PASTE_AND_GO);
 
   menu_contents->AddSeparator(ui::NORMAL_SEPARATOR);
 
   // Minor note: We use IDC_ for command id here while the underlying textfield
   // is using IDS_ for all its command ids. This is because views cannot depend
   // on IDC_ for now.
   menu_contents->AddItemWithStringId(IDC_EDIT_SEARCH_ENGINES,
       IDS_EDIT_SEARCH_ENGINES);
 }