A tricky fix for Issue 1845.

This change is a very tricky fix for Issue 1845 in chromium: cant alignt text to the right using right shift and right ctrl.This change consists of two parts listed below.

1. Emulating the implementation of Safari that changes the text-direction of an input element.

Safari uses context menus to change the text direction. This change adds an IPC message 'ViewMsg_SetTextDirection', which notifies the new text direction. Also, it adds two functions: RenderWidgetHost::UpdateTextDirection() and RenderWidgetHost::NotifyTextDirection(). They encapsulate the new IPC message so that we can use them both when we presses a set of keys and when we add context-menu items which change the text direction.

2. Calling the above interface when pressing right-shift and right-control keys, or when left-shift and left-control keys.

This modifies the RenderWidgetHostViewWin::OnKeyEvent() function and call the above text-direction interfaces when a user finishes pressing the keys. As you can imagine, if we send an IPC message every time when we receive a WM_KEYDOWN event, we continue sending IPC messages while a user is pressing the keys.

BUG=1845
Review URL: http://codereview.chromium.org/39252

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

15 files changed, 241 insertions, 1 deletions

diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc
index 83d7bcf..f40fe08 100644
--- a/chrome/browser/renderer_host/render_widget_host.cc
+++ b/chrome/browser/renderer_host/render_widget_host.cc
@@ -53,7 +53,9 @@ RenderWidgetHost::RenderWidgetHost(RenderProcessHost* process,
       mouse_move_pending_(false),
       needs_repainting_on_restore_(false),
       is_unresponsive_(false),
-      view_being_painted_(false) {
+      view_being_painted_(false),
+      text_direction_updated_(false),
+      text_direction_(WEB_TEXT_DIRECTION_LTR) {
   if (routing_id_ == MSG_ROUTING_NONE)
     routing_id_ = process_->GetNextRoutingID();
 
@@ -356,6 +358,19 @@ gfx::Rect RenderWidgetHost::GetRootWindowResizerRect() const {
   return gfx::Rect();
 }
 
+void RenderWidgetHost::UpdateTextDirection(WebTextDirection direction) {
+  text_direction_updated_ = true;
+  text_direction_ = direction;
+}
+
+void RenderWidgetHost::NotifyTextDirection() {
+  if (text_direction_updated_) {
+    text_direction_updated_ = false;
+    Send(new ViewMsg_SetTextDirection(routing_id(),
+                                      text_direction_));
+  }
+}
+
 void RenderWidgetHost::Destroy() {
   NotificationService::current()->Notify(
       NotificationType::RENDER_WIDGET_HOST_DESTROYED,
diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h
index 59235d0..01bd391 100644
--- a/chrome/browser/renderer_host/render_widget_host.h
+++ b/chrome/browser/renderer_host/render_widget_host.h
@@ -15,6 +15,7 @@
 #include "chrome/common/native_web_keyboard_event.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 #include "webkit/glue/webinputevent.h"
+#include "webkit/glue/webtextdirection.h"
 
 namespace gfx {
 class Rect;
@@ -215,6 +216,38 @@ class RenderWidgetHost : public IPC::Channel::Listener {
   // And to also expose it to the RenderWidgetHostView.
   virtual gfx::Rect GetRootWindowResizerRect() const;
 
+  // Update the text direction of the focused input element and notify it to a
+  // renderer process.
+  // These functions have two usage scenarios: changing the text direction
+  // from a menu (as Safari does), and; changing the text direction when a user
+  // presses a set of keys (as IE and Firefox do).
+  // 1. Change the text direction from a menu.
+  // In this scenario, we receive a menu event only once and we should update
+  // the text direction immediately when a user chooses a menu item. So, we
+  // should call both functions at once as listed in the following snippet.
+  //   void RenderViewHost::SetTextDirection(WebTextDirection direction) {
+  //     UpdateTextDirectioN(direction);
+  //     NotifyTextDirection();
+  //   }
+  // 2. Change the text direction when pressing a set of keys.
+  // Becauses of auto-repeat, we may receive the same key-press event many
+  // times while we presses the keys and it is nonsense to send the same IPC
+  // messsage every time when we receive a key-press event.
+  // To suppress the number of IPC messages, we just update the text direction
+  // when receiving a key-press event and send an IPC message when we release
+  // the keys as listed in the following snippet.
+  //   if (key_event.type == WebKeyboardEvent::KEY_DOWN) {
+  //     if (key_event.windows_key_code == 'A' &&
+  //         key_event.modifiers == WebKeyboardEvent::CTRL_KEY) {
+  //       UpdateTextDirectioN(dir);
+  //     }
+  //   } else if (key_event.type == WebKeyboardEvent::KEY_UP) {
+  //     NotifyTextDirection();
+  //   }
+  // Note: we cannot undo this change because either Firefox or IE cannot.
+  void UpdateTextDirection(WebTextDirection direction);
+  void NotifyTextDirection();
+
  protected:
   // Internal implementation of the public Forward*Event() methods.
   void ForwardInputEvent(const WebInputEvent& input_event, int event_size);
@@ -363,6 +396,10 @@ class RenderWidgetHost : public IPC::Channel::Listener {
   // back to whatever unhandled handler instead of the returned version.
   KeyQueue key_queue_;
 
+  // Set when we need to update the text direction.
+  bool text_direction_updated_;
+  WebTextDirection text_direction_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHost);
 };
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc
index 12dc1ba..52751ea 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_win.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc
@@ -823,6 +823,26 @@ LRESULT RenderWidgetHostViewWin::OnKeyEvent(UINT message, WPARAM wparam,
     return ::SendMessage(parent_hwnd_, message, wparam, lparam);
   }
 
+  if (wparam == VK_SHIFT || wparam == VK_CONTROL) {
+    // Bug 1845: we need to update the text direction when a user releases
+    // either a right-shift key or a right-control key after pressing both of
+    // them. So, we just update the text direction while a user is pressing the
+    // keys, and we notify the text direction when a user releases either of
+    // them.
+    if (message == WM_KEYDOWN) {
+      const int kKeyDownMask = 0x8000;
+      if ((GetKeyState(VK_RSHIFT) & kKeyDownMask) &&
+          (GetKeyState(VK_RCONTROL) & kKeyDownMask)) {
+        render_widget_host_->UpdateTextDirection(WEB_TEXT_DIRECTION_RTL);
+      } else if ((GetKeyState(VK_LSHIFT) & kKeyDownMask) &&
+                 (GetKeyState(VK_LCONTROL) & kKeyDownMask)) {
+        render_widget_host_->UpdateTextDirection(WEB_TEXT_DIRECTION_LTR);
+      }
+    } else if (message == WM_KEYUP) {
+      render_widget_host_->NotifyTextDirection();
+    }
+  }
+
   render_widget_host_->ForwardKeyboardEvent(
       NativeWebKeyboardEvent(m_hWnd, message, wparam, lparam));
   return 0;
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index ebac85c..4d67d88 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -38,6 +38,7 @@
 #include "webkit/glue/webinputevent.h"
 #include "webkit/glue/webplugin.h"
 #include "webkit/glue/webpreferences.h"
+#include "webkit/glue/webtextdirection.h"
 #include "webkit/glue/webview_delegate.h"
 
 #if defined(OS_POSIX)
@@ -1833,6 +1834,39 @@ struct ParamTraits<AudioOutputStream::State> {
   }
 };
 
+template <>
+struct ParamTraits<WebTextDirection> {
+  typedef WebTextDirection param_type;
+  static void Write(Message* m, const param_type& p) {
+    m->WriteInt(p);
+  }
+  static bool Read(const Message* m, void** iter, param_type* p) {
+    int type;
+    if (!m->ReadInt(iter, &type))
+      return false;
+    *p = static_cast<WebTextDirection>(type);
+    return true;
+  }
+  static void Log(const param_type& p, std::wstring* l) {
+    std::wstring control;
+    switch (p) {
+     case WEB_TEXT_DIRECTION_DEFAULT:
+      control = L"WEB_TEXT_DIRECTION_DEFAULT";
+      break;
+     case WEB_TEXT_DIRECTION_RTL:
+      control = L"WEB_TEXT_DIRECTION_RTL";
+      break;
+     case WEB_TEXT_DIRECTION_LTR:
+      control = L"WEB_TEXT_DIRECTION_LTR";
+      break;
+     default:
+      control = L"UNKNOWN";
+      break;
+    }
+
+    LogParam(control, l);
+  }
+};
 
 }  // namespace IPC
 
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index dedf4774..98006fd 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -24,6 +24,7 @@
 #include "webkit/glue/webcursor.h"
 #include "webkit/glue/webinputevent.h"
 #include "webkit/glue/webplugin.h"
+#include "webkit/glue/webtextdirection.h"
 
 // TODO(mpcomplete): rename ViewMsg and ViewHostMsg to something that makes
 // more sense with our current design.
@@ -511,6 +512,18 @@ IPC_BEGIN_MESSAGES(View)
   // Notification that a move or resize renderer's containing window has
   // started.
   IPC_MESSAGE_ROUTED0(ViewMsg_MoveOrResizeStarted)
+
+  // Changes the text direction of a selected input field.
+  // * direction (WebTextDirection)
+  //   Represents the new text direction.
+  //   Its possible values are listed below:
+  //     Value                      New Text Direction
+  //     WEB_TEXT_DIRECTION_DEFAULT NaturalWritingDirection ("inherit")
+  //     WEB_TEXT_DIRECTION_LTR     LeftToRightWritingDirection ("rtl")
+  //     WEB_TEXT_DIRECTION_RTL     RightToLeftWritingDirection ("ltr")
+  IPC_MESSAGE_ROUTED1(ViewMsg_SetTextDirection,
+                      WebTextDirection /* direction */)
+
 IPC_END_MESSAGES(View)
 
 
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 7263cb7..e60e301 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -374,6 +374,7 @@ class RenderView : public RenderWidget,
   FRIEND_TEST(RenderViewTest, OnNavStateChanged);
   FRIEND_TEST(RenderViewTest, OnImeStateChanged);
   FRIEND_TEST(RenderViewTest, ImeComposition);
+  FRIEND_TEST(RenderViewTest, OnSetTextDirection);
 
   explicit RenderView(RenderThreadBase* render_thread);
 
diff --git a/chrome/renderer/render_view_unittest.cc b/chrome/renderer/render_view_unittest.cc
index e81cf4f..a643052 100644
--- a/chrome/renderer/render_view_unittest.cc
+++ b/chrome/renderer/render_view_unittest.cc
@@ -317,3 +317,51 @@ TEST_F(RenderViewTest, ImeComposition) {
     }
   }
 }
+
+// Test that the RenderView::OnSetTextDirection() function can change the text
+// direction of the selected input element.
+TEST_F(RenderViewTest, OnSetTextDirection) {
+  // Load an HTML page consisting of a <textarea> element and a <div> element.
+  // This test changes the text direction of the <textarea> element, and
+  // writes the values of its 'dir' attribute and its 'direction' property to
+  // verify that the text direction is changed.
+  view_->set_delay_seconds_for_form_state_sync(0);
+  LoadHTML("<html>"
+           "<head>"
+           "</head>"
+           "<body>"
+           "<textarea id=\"test\"></textarea>"
+           "<div id=\"result\" contenteditable=\"true\"></div>"
+           "</body>"
+           "</html>");
+  render_thread_.sink().ClearMessages();
+
+  static const struct {
+    WebTextDirection direction;
+    const wchar_t* expected_result;
+  } kTextDirection[] = {
+    {WEB_TEXT_DIRECTION_RTL, L"\x000A" L"rtl,rtl"},
+    {WEB_TEXT_DIRECTION_LTR, L"\x000A" L"ltr,ltr"},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTextDirection); ++i) {
+    // Set the text direction of the <textarea> element.
+    ExecuteJavaScript("document.getElementById('test').focus();");
+    view_->OnSetTextDirection(kTextDirection[i].direction);
+
+    // Write the values of its DOM 'dir' attribute and its CSS 'direction'
+    // property to the <div> element.
+    ExecuteJavaScript("var result = document.getElementById('result');"
+                      "var node = document.getElementById('test');"
+                      "var style = getComputedStyle(node, null);"
+                      "result.innerText ="
+                      "    node.getAttribute('dir') + ',' +"
+                      "    style.getPropertyValue('direction');");
+
+    // Copy the document content to std::wstring and compare with the
+    // expected result.
+    const int kMaxOutputCharacters = 16;
+    std::wstring output;
+    GetMainFrame()->GetContentAsPlainText(kMaxOutputCharacters, &output);
+    EXPECT_EQ(output, kTextDirection[i].expected_result);
+  }
+}
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index a7ad380..6bfcb7b 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -119,6 +119,7 @@ IPC_DEFINE_MESSAGE_MAP(RenderWidget)
   IPC_MESSAGE_HANDLER(ViewMsg_ImeSetInputMode, OnImeSetInputMode)
   IPC_MESSAGE_HANDLER(ViewMsg_ImeSetComposition, OnImeSetComposition)
   IPC_MESSAGE_HANDLER(ViewMsg_Repaint, OnMsgRepaint)
+  IPC_MESSAGE_HANDLER(ViewMsg_SetTextDirection, OnSetTextDirection)
   IPC_MESSAGE_UNHANDLED_ERROR()
 IPC_END_MESSAGE_MAP()
 
@@ -635,6 +636,13 @@ void RenderWidget::OnMsgRepaint(const gfx::Size& size_to_paint) {
   DidInvalidateRect(webwidget_, repaint_rect);
 }
 
+void RenderWidget::OnSetTextDirection(WebTextDirection direction) {
+  if (!webwidget_)
+    return;
+
+  webwidget_->SetTextDirection(direction);
+}
+
 bool RenderWidget::next_paint_is_resize_ack() const {
   return ViewHostMsg_PaintRect_Flags::is_resize_ack(next_paint_flags_);
 }
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index 4079a8b..039b4da 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -19,6 +19,7 @@
 
 #include "webkit/glue/webwidget_delegate.h"
 #include "webkit/glue/webcursor.h"
+#include "webkit/glue/webtextdirection.h"
 
 class RenderThreadBase;
 struct WebPluginGeometry;
@@ -128,6 +129,7 @@ class RenderWidget : public IPC::Channel::Listener,
                            int target_start, int target_end,
                            const std::wstring& ime_string);
   void OnMsgRepaint(const gfx::Size& size_to_paint);
+  void OnSetTextDirection(WebTextDirection direction);
 
   // True if a PaintRect_ACK message is pending.
   bool paint_reply_pending() const {
diff --git a/webkit/glue/webtextdirection.h b/webkit/glue/webtextdirection.h
new file mode 100644
index 0000000..822eb34
--- /dev/null
+++ b/webkit/glue/webtextdirection.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2006-2008 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.
+
+#ifndef WEBKIT_GLUE_WEBTEXTDIRECTION_H_
+#define WEBKIT_GLUE_WEBTEXTDIRECTION_H_
+
+// Represents text directions (or writing directions) of a DOM node.
+// This type is used as the input parameter of WebWidget::SetTextDirection().
+// This function converts these values to WebCore::WritingDirection values and
+// call the Editor::setBaseWritingDirection() function.
+// TODO(hbono): Add WEB_TEXT_DIRECTION_ORIGINAL that represents "revert the
+// previous changes and set back to the original one" and implement it.
+// TODO(hbono): Add WEB_TEXT_DIRECTION_TOGGLE that represents "toggle the text
+// direction" and implement it.
+enum WebTextDirection {
+  WEB_TEXT_DIRECTION_DEFAULT,  // WebCore::NaturalWritingDirection
+  WEB_TEXT_DIRECTION_LTR,      // WebCore::LeftToRightWritingDirection
+  WEB_TEXT_DIRECTION_RTL,      // WebCore::RightToLeftWritingDirection
+};
+
+#endif  // WEBKIT_GLUE_WEBTEXTDIRECTION_H_
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index 69ca561..27e1842 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -1192,6 +1192,37 @@ bool WebViewImpl::ImeUpdateStatus(bool* enable_ime,
   return true;
 }
 
+void WebViewImpl::SetTextDirection(WebTextDirection direction) {
+  // The Editor::setBaseWritingDirection() function checks if we can change
+  // the text direction of the selected node and updates its DOM "dir"
+  // attribute and its CSS "direction" property.
+  // So, we just call the function as Safari does.
+  const Frame* focused = GetFocusedWebCoreFrame();
+  if (!focused)
+    return;
+  Editor* editor = focused->editor();
+  if (!editor || !editor->canEdit())
+    return;
+
+  switch (direction) {
+    case WEB_TEXT_DIRECTION_DEFAULT:
+      editor->setBaseWritingDirection(WebCore::NaturalWritingDirection);
+      break;
+
+    case WEB_TEXT_DIRECTION_LTR:
+      editor->setBaseWritingDirection(WebCore::LeftToRightWritingDirection);
+      break;
+
+    case WEB_TEXT_DIRECTION_RTL:
+      editor->setBaseWritingDirection(WebCore::RightToLeftWritingDirection);
+      break;
+
+    default:
+      NOTIMPLEMENTED();
+      break;
+  }
+}
+
 void WebViewImpl::RestoreFocus() {
   if (last_focused_frame_.get()) {
     if (last_focused_frame_->page()) {
diff --git a/webkit/glue/webview_impl.h b/webkit/glue/webview_impl.h
index 78e0a63..80839a4 100644
--- a/webkit/glue/webview_impl.h
+++ b/webkit/glue/webview_impl.h
@@ -75,6 +75,7 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
                                  const std::wstring& ime_string);
   virtual bool ImeUpdateStatus(bool* enable_ime,
                                gfx::Rect* caret_rect);
+  virtual void SetTextDirection(WebTextDirection direction);
   virtual void StopLoading();
   virtual void SetBackForwardListSize(int size);
   virtual void RestoreFocus();
diff --git a/webkit/glue/webwidget.h b/webkit/glue/webwidget.h
index 23046e8..b1b011d 100644
--- a/webkit/glue/webwidget.h
+++ b/webkit/glue/webwidget.h
@@ -6,6 +6,7 @@
 #define WEBKIT_GLUE_WEBWIDGET_H__
 
 #include "skia/ext/platform_canvas.h"
+#include "webkit/glue/webtextdirection.h"
 
 namespace gfx {
 class Rect;
@@ -64,6 +65,9 @@ class WebWidget {
   // Retrieve the status of this widget required by IME APIs.
   virtual bool ImeUpdateStatus(bool* enable_ime, gfx::Rect* caret_rect) = 0;
 
+  // Changes the text direction of the selected input node.
+  virtual void SetTextDirection(WebTextDirection direction) = 0;
+
  protected:
   virtual ~WebWidget() {}
 
diff --git a/webkit/glue/webwidget_impl.cc b/webkit/glue/webwidget_impl.cc
index b745a66..2a8383f 100644
--- a/webkit/glue/webwidget_impl.cc
+++ b/webkit/glue/webwidget_impl.cc
@@ -202,6 +202,9 @@ bool WebWidgetImpl::ImeUpdateStatus(bool* enable_ime,
   return false;
 }
 
+void WebWidgetImpl::SetTextDirection(WebTextDirection direction) {
+}
+
 //-----------------------------------------------------------------------------
 // WebCore::HostWindow
 
diff --git a/webkit/glue/webwidget_impl.h b/webkit/glue/webwidget_impl.h
index efb1b65..e10d666 100644
--- a/webkit/glue/webwidget_impl.h
+++ b/webkit/glue/webwidget_impl.h
@@ -50,6 +50,7 @@ class WebWidgetImpl : public WebWidget,
                                  const std::wstring& ime_string);
   virtual bool ImeUpdateStatus(bool* enable_ime,
                                gfx::Rect* caret_rect);
+  virtual void SetTextDirection(WebTextDirection direction);
 
   // WebWidgetImpl
   void Init(WebCore::FramelessScrollView* widget, const gfx::Rect& bounds);