Unicode bidi mirroring characters are not correctly mirrored in textfield and omnibox (which use CRichEditCtrl) while *inputting*.

Since bidi mirroring characters are correctly mirrored when rendering the whole text, fix it by re-rendering the whole text every time the text changes.

NOTE: this change will mess up the undo queue. The continuous keystroke wont be treated as one undo event.
Every single keystroke is treated as a undo state.

Using the following as example:
1. paste string "abcd " into find-in-page;
2. type in 'e', 'f', and ' '.
3. paste string "xyz".

When undo, the string sequence will be:
"abcd ef xyz" => "abcd ef " ==> "abcd ef" ==> "abcd e" ==> "abcd " =>"".

Without the change, the string sequence is:
"abcd ef xyz" => "abcd ef " => "abcd " =>"".



BUG=46298
TEST=follow the steps described in the bug.
Review URL: http://codereview.chromium.org/2741007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49811 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 74 insertions, 12 deletions

diff --git a/views/controls/textfield/native_textfield_win.cc b/views/controls/textfield/native_textfield_win.cc
index 75c8325..f3edb9b4 100644
--- a/views/controls/textfield/native_textfield_win.cc
+++ b/views/controls/textfield/native_textfield_win.cc
@@ -59,6 +59,20 @@ NativeTextfieldWin::ScopedFreeze::~ScopedFreeze() {
   }
 }
 
+NativeTextfieldWin::ScopedSuspendUndo::ScopedSuspendUndo(
+    ITextDocument* text_object_model)
+    : text_object_model_(text_object_model) {
+  // Suspend Undo processing.
+  if (text_object_model_)
+    text_object_model_->Undo(tomSuspend, NULL);
+}
+
+NativeTextfieldWin::ScopedSuspendUndo::~ScopedSuspendUndo() {
+  // Resume Undo processing.
+  if (text_object_model_)
+    text_object_model_->Undo(tomResume, NULL);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // NativeTextfieldWin
 
@@ -314,7 +328,7 @@ void NativeTextfieldWin::ExecuteCommand(int command_id) {
     case IDS_APP_SELECT_ALL: SelectAll();  break;
     default:                 NOTREACHED(); break;
   }
-  OnAfterPossibleChange();
+  OnAfterPossibleChange(true);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -415,7 +429,11 @@ LRESULT NativeTextfieldWin::OnImeComposition(UINT message,
     }
   }
 
-  OnAfterPossibleChange();
+  // If we allow OnAfterPossibleChange() to redraw the text, it will do this by
+  // setting the edit's text directly, which can cancel the current IME
+  // composition or cause other adverse affects. So we set |should_redraw_text|
+  // to false.
+  OnAfterPossibleChange(false);
   return result;
 }
 
@@ -473,7 +491,7 @@ void NativeTextfieldWin::OnKeyDown(TCHAR key, UINT repeat_count, UINT flags) {
         ScopedFreeze freeze(this, GetTextObjectModel());
         OnBeforePossibleChange();
         Cut();
-        OnAfterPossibleChange();
+        OnAfterPossibleChange(true);
       }
       return;
 
@@ -497,7 +515,7 @@ void NativeTextfieldWin::OnKeyDown(TCHAR key, UINT repeat_count, UINT flags) {
         ScopedFreeze freeze(this, GetTextObjectModel());
         OnBeforePossibleChange();
         Paste();
-        OnAfterPossibleChange();
+        OnAfterPossibleChange(true);
       }
       return;
 
@@ -528,7 +546,7 @@ void NativeTextfieldWin::OnLButtonDblClk(UINT keys, const CPoint& point) {
   OnBeforePossibleChange();
   DefWindowProc(WM_LBUTTONDBLCLK, keys,
                 MAKELPARAM(ClipXCoordToVisibleText(point.x, false), point.y));
-  OnAfterPossibleChange();
+  OnAfterPossibleChange(true);
 }
 
 void NativeTextfieldWin::OnLButtonDown(UINT keys, const CPoint& point) {
@@ -545,7 +563,7 @@ void NativeTextfieldWin::OnLButtonDown(UINT keys, const CPoint& point) {
   DefWindowProc(WM_LBUTTONDOWN, keys,
                 MAKELPARAM(ClipXCoordToVisibleText(point.x, is_triple_click),
                            point.y));
-  OnAfterPossibleChange();
+  OnAfterPossibleChange(true);
 }
 
 void NativeTextfieldWin::OnLButtonUp(UINT keys, const CPoint& point) {
@@ -553,7 +571,7 @@ void NativeTextfieldWin::OnLButtonUp(UINT keys, const CPoint& point) {
   OnBeforePossibleChange();
   DefWindowProc(WM_LBUTTONUP, keys,
                 MAKELPARAM(ClipXCoordToVisibleText(point.x, false), point.y));
-  OnAfterPossibleChange();
+  OnAfterPossibleChange(true);
 }
 
 void NativeTextfieldWin::OnMouseLeave() {
@@ -619,7 +637,7 @@ void NativeTextfieldWin::OnMouseMove(UINT keys, const CPoint& point) {
     GetRect(&r);
     DefWindowProc(WM_MOUSEMOVE, keys,
                   MAKELPARAM(point.x, (r.bottom - r.top) / 2));
-    OnAfterPossibleChange();
+    OnAfterPossibleChange(true);
   }
 }
 
@@ -796,7 +814,15 @@ void NativeTextfieldWin::HandleKeystroke(UINT message,
       DefWindowProc(message, key, MAKELPARAM(repeat_count, flags));
     }
 
-    OnAfterPossibleChange();
+    // CRichEditCtrl automatically turns on IMF_AUTOKEYBOARD when the user
+    // inputs an RTL character, making it difficult for the user to control
+    // what language is set as they type. Force this off to make the edit's
+    // behavior more stable.
+    const int lang_options = SendMessage(EM_GETLANGOPTIONS, 0, 0);
+    if (lang_options & IMF_AUTOKEYBOARD)
+      SendMessage(EM_SETLANGOPTIONS, 0, lang_options & ~IMF_AUTOKEYBOARD);
+
+    OnAfterPossibleChange(true);
   }
 }
 
@@ -805,7 +831,7 @@ void NativeTextfieldWin::OnBeforePossibleChange() {
   text_before_change_ = GetText();
 }
 
-void NativeTextfieldWin::OnAfterPossibleChange() {
+void NativeTextfieldWin::OnAfterPossibleChange(bool should_redraw_text) {
   // Prevent the user from selecting the "phantom newline" at the end of the
   // edit.  If they try, we just silently move the end of the selection back to
   // the end of the real text.
@@ -835,6 +861,17 @@ void NativeTextfieldWin::OnAfterPossibleChange() {
     textfield_->SyncText();
     if (textfield_->GetController())
       textfield_->GetController()->ContentsChanged(textfield_, new_text);
+
+    if (should_redraw_text) {
+      CHARRANGE original_sel;
+      GetSel(original_sel);
+      std::wstring text = GetText();
+      ScopedSuspendUndo suspend_undo(GetTextObjectModel());
+
+      SelectAll();
+      ReplaceSel(reinterpret_cast<LPCTSTR>(text.c_str()), true);
+      SetSel(original_sel);
+    }
   }
 }
 
diff --git a/views/controls/textfield/native_textfield_win.h b/views/controls/textfield/native_textfield_win.h
index 496e332..f3b4d5e 100644
--- a/views/controls/textfield/native_textfield_win.h
+++ b/views/controls/textfield/native_textfield_win.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -118,6 +118,21 @@ class NativeTextfieldWin
     DISALLOW_COPY_AND_ASSIGN(ScopedFreeze);
   };
 
+  // This object suspends placing any operations on the edit's undo stack until
+  // the object is destroyed.  If we don't do this, some of the operations we
+  // perform behind the user's back will be undoable by the user, which feels
+  // bizarre and confusing.
+  class ScopedSuspendUndo {
+   public:
+    explicit ScopedSuspendUndo(ITextDocument* text_object_model);
+    ~ScopedSuspendUndo();
+
+   private:
+    ITextDocument* const text_object_model_;
+
+    DISALLOW_COPY_AND_ASSIGN(ScopedSuspendUndo);
+  };
+
   // message handlers
   void OnChar(TCHAR key, UINT repeat_count, UINT flags);
   void OnContextMenu(HWND window, const POINT& point);
@@ -149,7 +164,17 @@ class NativeTextfieldWin
   // before and after the change.  These functions determine if anything
   // meaningful changed, and do any necessary updating and notification.
   void OnBeforePossibleChange();
-  void OnAfterPossibleChange();
+
+  // When a user types a BIDI mirroring character (e.g. left parenthesis
+  // U+0028, which should be rendered as '(' in LTR context unless  surrounded
+  // by RTL characters in both sides, and it should be rendered as ')' in RTL
+  // context unless surrounded by LTR characters in both sides.), the textfield
+  // does not properly mirror the character when necessary. However, if we
+  // explicitly set the text in the edit to the entire current string, then
+  // the BIDI mirroring characters will be mirrored properly. When
+  // |should_redraw_text| is true, we explicitly set the text in the edit to
+  // the entire current string any time the text changes.
+  void OnAfterPossibleChange(bool should_redraw_text);
 
   // Given an X coordinate in client coordinates, returns that coordinate
   // clipped to be within the horizontal bounds of the visible text.