views: update textfield cursor state after programmatic text changes

ui/views/controls/textfield/textfield.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 "ui/views/controls/textfield/textfield.h"
 
 #include <string>
 #include <utility>
 
 #include "base/trace_event/trace_event.h"
@@ skipping 289 matching lines @@
   SchedulePaint();
 }
 
 void Textfield::SetTextInputFlags(int flags) {
   text_input_flags_ = flags;
 }
 
 void Textfield::SetText(const base::string16& new_text) {
   model_->SetText(new_text);
   OnCaretBoundsChanged();
+  UpdateAfterChange(true, true);

[Peter Kasting, 2016/09/28 18:56:47]

UpdateAfterChange() calls OnCaretBoundsChanged(), SchedulePaint(), and
NotifyAccessibilityEvent(), so those calls are now unnecessary (and, in the
accessibility case, probably outright wrong).  (multiple places)

This will now notify the controller about the text contents changing, where it
didn't before.  Do we need to worry about this, e.g. some kind of infinite loop
or double-handling of changes because whoever is calling us expected not to be
notified?

[Elly Fong-Jones, 2016/10/03 17:37:30]

Done.
I don't know. The behavior isn't documented one way or the other.

Looking at the callsites for these functions:

1) ::SetText() is used in a zillion places. It's probably not feasible to check
each callsite.
2) ::AppendText() is used only in Views unittests, so if it's broken, we'll find
out from the tests.
3) ::InsertOrReplaceText() is used in only one place:
OmniboxViewViews::OnPaste(). I looked at that code and tested it, and it seems
to work okay.

So, I'm not sure what to do about SetText(). Maybe we don't need to
UpdateAfterChange() there, but I don't know how to find out.

   SchedulePaint();
   NotifyAccessibilityEvent(ui::AX_EVENT_TEXT_CHANGED, true);
 }
 
 void Textfield::AppendText(const base::string16& new_text) {
   if (new_text.empty())
     return;
   model_->Append(new_text);
   OnCaretBoundsChanged();
+  UpdateAfterChange(true, true);

[Peter Kasting, 2016/09/28 18:56:47]

The next two functions now trigger accessibility notifications that didn't
before.  What effect will this have on e.g. screen readers?  This may be a
bugfix, or we may now get annoying notifications we didn't care about.

[Elly Fong-Jones, 2016/10/03 17:37:30]

I don't know. There are several screenreaders, all with different behavior and
handling of different notifications, so it's just about impossible to say for
certain whether this change is good or bad. :\

   SchedulePaint();
 }
 
 void Textfield::InsertOrReplaceText(const base::string16& new_text) {
   if (new_text.empty())
     return;
   model_->InsertText(new_text);
   OnCaretBoundsChanged();
+  UpdateAfterChange(true, true);
   SchedulePaint();
 }
 
 base::string16 Textfield::GetSelectedText() const {
   return model_->GetSelectedText();
 }
 
 void Textfield::SelectAll(bool reversed) {
   model_->SelectAll(reversed);
   UpdateSelectionClipboard();
@@ skipping 1722 matching lines @@
 }
 
 void Textfield::OnCursorBlinkTimerFired() {
   DCHECK(ShouldBlinkCursor());
   gfx::RenderText* render_text = GetRenderText();
   render_text->set_cursor_visible(!render_text->cursor_visible());
   RepaintCursor();
 }
 
 }  // namespace views