views: update textfield cursor state after programmatic text changes

ui/views/controls/textfield/textfield.cc

Index: ui/views/controls/textfield/textfield.cc
diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc
index 4642d23fc89a89633cc84ff72fbb5011959a7b9d..3819c48cb3afec87c2e5950b350617e43addc332 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -307,6 +307,7 @@ void Textfield::SetTextInputFlags(int 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);
 }
@@ -316,6 +317,7 @@ void Textfield::AppendText(const base::string16& new_text) {
     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();
 }
 
@@ -324,6 +326,7 @@ void Textfield::InsertOrReplaceText(const base::string16& new_text) {
     return;
   model_->InsertText(new_text);
   OnCaretBoundsChanged();
+  UpdateAfterChange(true, true);
   SchedulePaint();
 }