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

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

Index: chrome/browser/ui/views/omnibox/omnibox_view_views.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index c9cf7111b946dbc0c44ac718eed6eca6bb15480c..f47226ec5d07b7c5e4690c9f74035247351067b4 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -419,6 +419,23 @@ 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);
 }
@@ -742,6 +759,12 @@ void OmniboxViewViews::GetAccessibleState(ui::AXViewState* state) {
   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;
 
@@ -752,6 +775,9 @@ void OmniboxViewViews::GetAccessibleState(ui::AXViewState* state) {
     state->set_value_callback =
         base::Bind(&OmniboxViewViews::AccessibilitySetValue,
                    weak_ptr_factory_.GetWeakPtr());
+    state->replace_selection_callback =
+        base::Bind(&OmniboxViewViews::AccessibilityReplaceSelection,
+                   weak_ptr_factory_.GetWeakPtr());
   }
 }