[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..f4df38168214bcbbfba1f71f6ecc9c8ced2a8089 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)

[Peter Kasting, 2013/12/03 22:52:35]

These #includes should not be ifdefed.

[pals, 2013/12/04 12:54:50]

Done.

+#include "content/public/browser/render_view_host.h"
+#include "ui/base/l10n/l10n_util.h"
+#endif // !OS_WIN
+
 using content::WebContents;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -25,7 +30,8 @@ using content::WebContents;
 RenderViewContextMenuViews::RenderViewContextMenuViews(
     WebContents* web_contents,
     const content::ContextMenuParams& params)
-    : RenderViewContextMenu(web_contents, params) {
+    : RenderViewContextMenu(web_contents, params),
+      bidi_submenu_model_(this) {
 }
 
 RenderViewContextMenuViews::~RenderViewContextMenuViews() {
@@ -109,6 +115,87 @@ bool RenderViewContextMenuViews::GetAcceleratorForCommandId(
   }
 }
 
+void RenderViewContextMenuViews::ExecuteCommand(int command_id,
+    int event_flags) {

[Peter Kasting, 2013/12/03 22:52:35]

Nit: Indent even

[pals, 2013/12/04 12:54:50]

Done.

+  switch (command_id) {
+    case IDC_WRITING_DIRECTION_DEFAULT:
+      // WebKit's current behavior is for this menu item to always be disabled.

[Peter Kasting, 2013/12/03 22:52:35]

Then why are we displaying it in the menu?  Or will this be changing imminently?

[pals, 2013/12/04 12:54:50]

I think default is for dir="auto". see crbug.com/317739. Not sure why it is
always be disabled. Mac chrome has same implementation.

[Peter Kasting, 2013/12/04 19:46:31]

Right, I'd also think it corresponds to dir=auto.  But there's no obvious reason
why that shouldn't be available in the menu.  Can you investigate why this is
always disabled so we can understand whether to remove it from the menu for now
or just make it enabled at the appropriate times?

[pals, 2013/12/05 14:25:49]

I found original implementaion on OSX (webkit side
https://bugs.webkit.org/show_bug.cgi?id=34524 and chromium side
https://codereview.chromium.org/566031) which does not explain much about this
behavior. (I have also sent a mail to jeremy, waiting for him to get back on
this)
and I came across this
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit....
We can enable "default" for Natural writing direction ("inherit").

[jeremy, 2013/12/05 14:33:18]

If I had to guess, I'd think this is a "match-the-mac-platform-ism" leftover
from WebKit, you can find documentation on it here:
http://support.apple.com/kb/PH11211 .

Aharon would be able to weigh in on whether this item makes sense on other
platforms.

+      NOTREACHED();
+      break;
+
+    case IDC_WRITING_DIRECTION_RTL:
+    case IDC_WRITING_DIRECTION_LTR: {
+      content::RenderViewHost* view_host = GetRenderViewHost();
+      view_host->UpdateTextDirection((command_id == IDC_WRITING_DIRECTION_RTL) ?
+          blink::WebTextDirectionRightToLeft :
+          blink::WebTextDirectionLeftToRight);
+      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/12/03 22:52:35]

This comment seems out of date.

[pals, 2013/12/04 12:54:50]

Done.

+  AppendBidiSubMenu();

[Peter Kasting, 2013/12/03 22:52:35]

Why a separate function for this instead of just doing the relevant work here?

[pals, 2013/12/04 12:54:50]

Done.

+}
+
+void RenderViewContextMenuViews::AppendBidiSubMenu() {
+  bidi_submenu_model_.AddCheckItem(IDC_WRITING_DIRECTION_DEFAULT,

[Peter Kasting, 2013/12/03 22:52:35]

Nit: Wrap after '(' so all lines of args are aligned (3 places)

[pals, 2013/12/04 12:54:50]

Done.

+      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_);
+}
+
 void RenderViewContextMenuViews::UpdateMenuItem(int command_id,
                                                 bool enabled,
                                                 bool hidden,