[Aura] Adding Writing direction to the context menu for Aura.

chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc

Index: chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
diff --git a/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc b/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
index 19294bacc477e2bc632e3947051c3a1a2cacb11b..f6e08d8209741d2f66b8c0a3b868c848d9034701 100644
--- a/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
+++ b/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
@@ -17,6 +17,11 @@
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_runner.h"
 
+#if !defined(OS_WIN)
+#include "content/public/browser/render_view_host.h"
+#include "ui/base/l10n/l10n_util.h"
+#endif // !OS_WIN
+
 using content::WebContents;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -25,7 +30,11 @@ using content::WebContents;
 RenderViewContextMenuViews::RenderViewContextMenuViews(
     WebContents* web_contents,
     const content::ContextMenuParams& params)
-    : RenderViewContextMenu(web_contents, params) {
+    : RenderViewContextMenu(web_contents, params)
+#if !defined(OS_WIN)
+      , bidi_submenu_model_(this)
+#endif // !OS_WIN
+{
 }
 
 RenderViewContextMenuViews::~RenderViewContextMenuViews() {
@@ -109,6 +118,90 @@ bool RenderViewContextMenuViews::GetAcceleratorForCommandId(
   }
 }
 
+#if !defined(OS_WIN)
+void RenderViewContextMenuViews::ExecuteCommand(
+    int command_id, int event_flags) {

[Peter Kasting, 2013/11/22 21:53:03]

Nit: One arg per line; try to put the first arg on the line above if possible

[pals, 2013/11/25 06:28:13]

Done.

+  switch (command_id) {
+    case IDC_WRITING_DIRECTION_DEFAULT:
+      // WebKit's current behavior is for this menu item to always be disabled.
+      NOTREACHED();
+      break;
+
+    case IDC_WRITING_DIRECTION_RTL:
+    case IDC_WRITING_DIRECTION_LTR: {
+      content::RenderViewHost* view_host = GetRenderViewHost();
+      blink::WebTextDirection dir = blink::WebTextDirectionLeftToRight;
+      if (command_id == IDC_WRITING_DIRECTION_RTL)
+        dir = blink::WebTextDirectionRightToLeft;
+      view_host->UpdateTextDirection(dir);

[Peter Kasting, 2013/11/22 21:53:03]

Nit: Shorter and simpler:

      view_host->UpdateTextDirection((command_id == IDC_WRITING_DIRECTION_RTL) ?
          blink::WebTextDirectionRightToLeft :
          blink::WebTextDirectionLeftToRight);

[pals, 2013/11/25 06:28:13]

Done. looks better.

+      view_host->NotifyTextDirection();
+      break;
+    }
+
+    default:
+      RenderViewContextMenu::ExecuteCommand(command_id, event_flags);
+      break;
+  }
+}
+
+bool RenderViewContextMenuViews::IsCommandIdChecked(int command_id) const {
+  switch (command_id) {
+    case IDC_WRITING_DIRECTION_DEFAULT:
+      return params_.writing_direction_default &
+          blink::WebContextMenuData::CheckableMenuItemChecked;
+    case IDC_WRITING_DIRECTION_RTL:
+      return params_.writing_direction_right_to_left &
+          blink::WebContextMenuData::CheckableMenuItemChecked;
+    case IDC_WRITING_DIRECTION_LTR:
+      return params_.writing_direction_left_to_right &
+          blink::WebContextMenuData::CheckableMenuItemChecked;
+
+    default:
+      return RenderViewContextMenu::IsCommandIdChecked(command_id);
+  }
+}
+
+bool RenderViewContextMenuViews::IsCommandIdEnabled(int command_id) const {
+  switch (command_id) {
+    case IDC_WRITING_DIRECTION_MENU:
+      return true;
+    case IDC_WRITING_DIRECTION_DEFAULT:  // Provided to match OS defaults.
+      return params_.writing_direction_default &
+          blink::WebContextMenuData::CheckableMenuItemEnabled;
+    case IDC_WRITING_DIRECTION_RTL:
+      return params_.writing_direction_right_to_left &
+          blink::WebContextMenuData::CheckableMenuItemEnabled;
+    case IDC_WRITING_DIRECTION_LTR:
+      return params_.writing_direction_left_to_right &
+          blink::WebContextMenuData::CheckableMenuItemEnabled;
+
+    default:
+      return RenderViewContextMenu::IsCommandIdEnabled(command_id);
+  }
+}
+
+void RenderViewContextMenuViews::AppendPlatformEditableItems() {
+  // OS X and Linux provide a contextual menu to set writing direction for BiDi
+  // languages.
+  // This functionality is exposed as a keyboard shortcut on Windows.

[Peter Kasting, 2013/11/22 21:53:03]

We need to have a keyboard shortcut on all platforms, context menu or not.

If you are only adding this menu because you don't have a shortcut, add a
shortcut instead of adding this menu.

[pals, 2013/11/25 06:28:13]

As per the comment #31
http://code.google.com/p/chromium/issues/detail?id=91178#c31, I thought of
exposing this as context menu similar to MAC and Linux.

[Peter Kasting, 2013/11/26 02:18:31]

I'm not necessarily opposed to having a context menu at all, but context menus
should always be a secondary way of triggering some functionality.  It seems
like adding a keyboard shortcut should happen either first or at least alongside
this patch.

Also why wouldn't we be consistent and expose the context menu here on Windows
as well?  It doesn't make sense to me to implement this only for some platforms.
 Or is such an option already exposed in the Windows context menu some other
way?

+  AppendBidiSubMenu();
+}
+
+void RenderViewContextMenuViews::AppendBidiSubMenu() {
+  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_DEFAULT,
+      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_DEFAULT));
+  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_LTR,
+      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_LTR));
+  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_RTL,
+      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_RTL));
+
+  menu_model_.AddSubMenu(
+      IDC_WRITING_DIRECTION_MENU,
+      l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_WRITING_DIRECTION_MENU),
+      &bidi_submenu_model_);
+}
+#endif // !OS_WIN
+
 void RenderViewContextMenuViews::UpdateMenuItem(int command_id,
                                                 bool enabled,
                                                 bool hidden,