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

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

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h"
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "grit/generated_resources.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/point.h"
 #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;
 
 ////////////////////////////////////////////////////////////////////////////////
 // RenderViewContextMenuViews, public:
 
 RenderViewContextMenuViews::RenderViewContextMenuViews(
     WebContents* web_contents,
     const content::ContextMenuParams& params)
-    : RenderViewContextMenu(web_contents, params) {
+    : RenderViewContextMenu(web_contents, params),
+      bidi_submenu_model_(this) {
 }
 
 RenderViewContextMenuViews::~RenderViewContextMenuViews() {
 }
 
 #if !defined(OS_WIN)
 // static
 RenderViewContextMenuViews* RenderViewContextMenuViews::Create(
     content::WebContents* web_contents,
     const content::ContextMenuParams& params) {
@@ skipping 63 matching lines @@
 
     case IDC_CONTENT_CONTEXT_SELECTALL:
       *accel = ui::Accelerator(ui::VKEY_A, ui::EF_CONTROL_DOWN);
       return true;
 
     default:
       return false;
   }
 }
 
+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,
                                                 const string16& title) {
   views::MenuItemView* item =
       menu_runner_->GetMenu()->GetMenuItemByID(command_id);
   if (!item)
     return;
 
   item->SetEnabled(enabled);
   item->SetTitle(title);
   item->SetVisible(!hidden);
 
   views::MenuItemView* parent = item->GetParentMenuItem();
   if (!parent)
     return;
 
   parent->ChildrenChanged();
 }