From 7ee860dcbc368df75c80246fa0d3cc2cca27aa5e Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sat, 20 Oct 2018 02:29:12 +0200
Subject: [PATCH 01/10] refactor: improve menubar keyboard accessibility

---
 atom/browser/ui/views/menu_bar.cc       | 56 +++++++++++++++++++++----
 atom/browser/ui/views/menu_bar.h        | 18 ++++----
 atom/browser/ui/views/menu_delegate.cc  | 23 ++++++++--
 atom/browser/ui/views/menu_delegate.h   | 17 +++++++-
 atom/browser/ui/views/root_view.cc      | 17 +++++++-
 atom/browser/ui/views/root_view.h       |  4 ++
 atom/browser/ui/views/submenu_button.cc |  5 +++
 atom/browser/ui/views/submenu_button.h  |  3 ++
 8 files changed, 123 insertions(+), 20 deletions(-)

diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc
index 332a42b513a1e..6e598cb710299 100644
--- a/atom/browser/ui/views/menu_bar.cc
+++ b/atom/browser/ui/views/menu_bar.cc
@@ -7,11 +7,11 @@
 #include <memory>
 #include <string>
 
-#include "atom/browser/ui/views/menu_delegate.h"
 #include "atom/browser/ui/views/submenu_button.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/views/background.h"
 #include "ui/views/layout/box_layout.h"
+#include "ui/views/widget/widget.h"
 
 #if defined(USE_X11)
 #include "chrome/browser/ui/libgtkui/gtk_util.h"
@@ -32,18 +32,16 @@ const SkColor kDefaultColor = SkColorSetARGB(255, 233, 233, 233);
 
 const char MenuBar::kViewClassName[] = "ElectronMenuBar";
 
-MenuBar::MenuBar(views::View* window)
+MenuBar::MenuBar(RootView* window)
     : background_color_(kDefaultColor), window_(window) {
   RefreshColorCache();
   UpdateViewColors();
+  SetFocusBehavior(FocusBehavior::ALWAYS);
   SetLayoutManager(
       std::make_unique<views::BoxLayout>(views::BoxLayout::kHorizontal));
-  window_->GetFocusManager()->AddFocusChangeListener(this);
 }
 
-MenuBar::~MenuBar() {
-  window_->GetFocusManager()->RemoveFocusChangeListener(this);
-}
+MenuBar::~MenuBar() {}
 
 void MenuBar::SetMenu(AtomMenuModel* model) {
   menu_model_ = model;
@@ -96,6 +94,41 @@ bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& screenPoint,
   return false;
 }
 
+void MenuBar::OnBeforeExecuteCommand() {
+  RemovePaneFocus();
+  window_->RestoreFocus();
+}
+
+bool MenuBar::AcceleratorPressed(const ui::Accelerator& accelerator) {
+  views::View* focused_view = GetFocusManager()->GetFocusedView();
+  if (!ContainsForFocusSearch(this, focused_view))
+    return false;
+
+  switch (accelerator.key_code()) {
+    case ui::VKEY_ESCAPE: {
+      RemovePaneFocus();
+      window_->RestoreFocus();
+      return true;
+    }
+    case ui::VKEY_LEFT:
+      GetFocusManager()->AdvanceFocus(true);
+      return true;
+    case ui::VKEY_RIGHT:
+      GetFocusManager()->AdvanceFocus(false);
+      return true;
+    case ui::VKEY_HOME:
+      GetFocusManager()->SetFocusedViewWithReason(
+          GetFirstFocusableChild(), views::FocusManager::kReasonFocusTraversal);
+      return true;
+    case ui::VKEY_END:
+      GetFocusManager()->SetFocusedViewWithReason(
+          GetLastFocusableChild(), views::FocusManager::kReasonFocusTraversal);
+      return true;
+    default:
+      return false;
+  }
+}
+
 const char* MenuBar::GetClassName() const {
   return kViewClassName;
 }
@@ -119,9 +152,16 @@ void MenuBar::OnMenuButtonClicked(views::MenuButton* source,
     return;
   }
 
+  GetFocusManager()->SetFocusedViewWithReason(
+      source, views::FocusManager::kReasonFocusTraversal);
+
   // Deleted in MenuDelegate::OnMenuClosed
   MenuDelegate* menu_delegate = new MenuDelegate(this);
-  menu_delegate->RunMenu(menu_model_->GetSubmenuModelAt(id), source);
+  menu_delegate->RunMenu(menu_model_->GetSubmenuModelAt(id), source,
+                         event != nullptr && event->IsKeyEvent()
+                             ? ui::MENU_SOURCE_KEYBOARD
+                             : ui::MENU_SOURCE_MOUSE);
+  menu_delegate->AddObserver(this);
 }
 
 void MenuBar::RefreshColorCache(const ui::NativeTheme* theme) {
@@ -152,6 +192,8 @@ void MenuBar::OnNativeThemeChanged(const ui::NativeTheme* theme) {
 }
 
 void MenuBar::OnDidChangeFocus(View* focused_before, View* focused_now) {
+  views::AccessiblePaneView::OnDidChangeFocus(focused_before, focused_now);
+
   // if we've changed focus, update our view
   const auto had_focus = has_focus_;
   has_focus_ = focused_now != nullptr;
diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h
index 2b10ba3e3bf2c..5f30b91366838 100644
--- a/atom/browser/ui/views/menu_bar.h
+++ b/atom/browser/ui/views/menu_bar.h
@@ -6,6 +6,9 @@
 #define ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_
 
 #include "atom/browser/ui/atom_menu_model.h"
+#include "atom/browser/ui/views/menu_delegate.h"
+#include "atom/browser/ui/views/root_view.h"
+#include "ui/views/accessible_pane_view.h"
 #include "ui/views/controls/button/menu_button_listener.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/view.h"
@@ -16,15 +19,13 @@ class MenuButton;
 
 namespace atom {
 
-class MenuDelegate;
-
-class MenuBar : public views::View,
+class MenuBar : public views::AccessiblePaneView,
                 public views::MenuButtonListener,
-                public views::FocusChangeListener {
+                public atom::MenuDelegate::Observer {
  public:
   static const char kViewClassName[];
 
-  explicit MenuBar(views::View* window);
+  explicit MenuBar(RootView* window);
   ~MenuBar() override;
 
   // Replaces current menu with a new one.
@@ -47,6 +48,10 @@ class MenuBar : public views::View,
                                     AtomMenuModel** menu_model,
                                     views::MenuButton** button);
 
+  void OnBeforeExecuteCommand() override;
+
+  bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
+
  protected:
   // views::View:
   const char* GetClassName() const override;
@@ -59,7 +64,6 @@ class MenuBar : public views::View,
 
   // views::FocusChangeListener:
   void OnDidChangeFocus(View* focused_before, View* focused_now) override;
-  void OnWillChangeFocus(View* focused_before, View* focused_now) override {}
 
  private:
   void RebuildChildren();
@@ -72,7 +76,7 @@ class MenuBar : public views::View,
   SkColor disabled_color_;
 #endif
 
-  views::View* window_ = nullptr;
+  RootView* window_ = nullptr;
   AtomMenuModel* menu_model_ = nullptr;
 
   View* FindAccelChild(base::char16 key);
diff --git a/atom/browser/ui/views/menu_delegate.cc b/atom/browser/ui/views/menu_delegate.cc
index d8cb592293a76..9af27504d5de8 100644
--- a/atom/browser/ui/views/menu_delegate.cc
+++ b/atom/browser/ui/views/menu_delegate.cc
@@ -14,17 +14,24 @@
 
 namespace atom {
 
-MenuDelegate::MenuDelegate(MenuBar* menu_bar) : menu_bar_(menu_bar), id_(-1) {}
+MenuDelegate::MenuDelegate(MenuBar* menu_bar)
+    : menu_bar_(menu_bar), id_(-1), hold_first_switch_(false) {}
 
 MenuDelegate::~MenuDelegate() {}
 
-void MenuDelegate::RunMenu(AtomMenuModel* model, views::MenuButton* button) {
+void MenuDelegate::RunMenu(AtomMenuModel* model,
+                           views::MenuButton* button,
+                           ui::MenuSourceType source_type) {
   gfx::Point screen_loc;
   views::View::ConvertPointToScreen(button, &screen_loc);
   // Subtract 1 from the height to make the popup flush with the button border.
   gfx::Rect bounds(screen_loc.x(), screen_loc.y(), button->width(),
                    button->height() - 1);
 
+  if (source_type == ui::MENU_SOURCE_KEYBOARD) {
+    hold_first_switch_ = true;
+  }
+
   id_ = button->tag();
   adapter_.reset(new MenuModelAdapter(model));
 
@@ -35,15 +42,18 @@ void MenuDelegate::RunMenu(AtomMenuModel* model, views::MenuButton* button) {
       item,
       views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS));
   menu_runner_->RunMenuAt(button->GetWidget()->GetTopLevelWidget(), button,
-                          bounds, views::MENU_ANCHOR_TOPRIGHT,
-                          ui::MENU_SOURCE_MOUSE);
+                          bounds, views::MENU_ANCHOR_TOPRIGHT, source_type);
 }
 
 void MenuDelegate::ExecuteCommand(int id) {
+  for (Observer& obs : observers_)
+    obs.OnBeforeExecuteCommand();
   adapter_->ExecuteCommand(id);
 }
 
 void MenuDelegate::ExecuteCommand(int id, int mouse_event_flags) {
+  for (Observer& obs : observers_)
+    obs.OnBeforeExecuteCommand();
   adapter_->ExecuteCommand(id, mouse_event_flags);
 }
 
@@ -101,6 +111,11 @@ views::MenuItemView* MenuDelegate::GetSiblingMenu(
     views::MenuAnchorPosition* anchor,
     bool* has_mnemonics,
     views::MenuButton**) {
+  if (hold_first_switch_) {
+    hold_first_switch_ = false;
+    return nullptr;
+  }
+
   // TODO(zcbenz): We should follow Chromium's logics on implementing the
   // sibling menu switches, this code is almost a hack.
   views::MenuButton* button;
diff --git a/atom/browser/ui/views/menu_delegate.h b/atom/browser/ui/views/menu_delegate.h
index e23262a3aa7f6..efa38c6a742e8 100644
--- a/atom/browser/ui/views/menu_delegate.h
+++ b/atom/browser/ui/views/menu_delegate.h
@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "atom/browser/ui/atom_menu_model.h"
+#include "base/observer_list.h"
 #include "ui/views/controls/menu/menu_delegate.h"
 
 namespace views {
@@ -23,7 +24,18 @@ class MenuDelegate : public views::MenuDelegate {
   explicit MenuDelegate(MenuBar* menu_bar);
   ~MenuDelegate() override;
 
-  void RunMenu(AtomMenuModel* model, views::MenuButton* button);
+  void RunMenu(AtomMenuModel* model,
+               views::MenuButton* button,
+               ui::MenuSourceType source_type);
+
+  class Observer {
+   public:
+    virtual void OnBeforeExecuteCommand() = 0;
+  };
+
+  void AddObserver(Observer* obs) { observers_.AddObserver(obs); }
+
+  void RemoveObserver(const Observer* obs) { observers_.RemoveObserver(obs); }
 
  protected:
   // views::MenuDelegate:
@@ -55,6 +67,9 @@ class MenuDelegate : public views::MenuDelegate {
 
   // The menu button to switch to.
   views::MenuButton* button_to_open_ = nullptr;
+  bool hold_first_switch_;
+
+  base::ObserverList<Observer> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuDelegate);
 };
diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc
index 716563ff0141c..e6c48048ee0d1 100644
--- a/atom/browser/ui/views/root_view.cc
+++ b/atom/browser/ui/views/root_view.cc
@@ -35,7 +35,9 @@ bool IsAltModifier(const content::NativeWebKeyboardEvent& event) {
 
 }  // namespace
 
-RootView::RootView(NativeWindow* window) : window_(window) {
+RootView::RootView(NativeWindow* window)
+    : window_(window),
+      last_focused_view_tracker_(std::make_unique<views::ViewTracker>()) {
   set_owned_by_client();
 }
 
@@ -140,12 +142,25 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) {
     // When a single Alt is released right after a Alt is pressed:
     menu_bar_alt_pressed_ = false;
     SetMenuBarVisibility(!menu_bar_visible_);
+    View* focused_view = GetFocusManager()->GetFocusedView();
+    last_focused_view_tracker_->SetView(focused_view);
+    menu_bar_->RequestFocus();
   } else {
     // When any other keys except single Alt have been pressed/released:
     menu_bar_alt_pressed_ = false;
   }
 }
 
+void RootView::RestoreFocus() {
+  View* last_focused_view = last_focused_view_tracker_->view();
+  if (last_focused_view) {
+    GetFocusManager()->SetFocusedViewWithReason(
+        last_focused_view, views::FocusManager::kReasonFocusRestore);
+  }
+  if (menu_bar_autohide_)
+    SetMenuBarVisibility(false);
+}
+
 void RootView::ResetAltState() {
   menu_bar_alt_pressed_ = false;
 }
diff --git a/atom/browser/ui/views/root_view.h b/atom/browser/ui/views/root_view.h
index 66c6d4e8a5571..b5b718f28fc9b 100644
--- a/atom/browser/ui/views/root_view.h
+++ b/atom/browser/ui/views/root_view.h
@@ -9,6 +9,7 @@
 
 #include "atom/browser/ui/accelerator_util.h"
 #include "ui/views/view.h"
+#include "ui/views/view_tracker.h"
 
 namespace content {
 struct NativeWebKeyboardEvent;
@@ -34,6 +35,7 @@ class RootView : public views::View {
   bool IsMenuBarVisible() const;
   void HandleKeyEvent(const content::NativeWebKeyboardEvent& event);
   void ResetAltState();
+  void RestoreFocus();
 
   // views::View:
   void Layout() override;
@@ -57,6 +59,8 @@ class RootView : public views::View {
   // Map from accelerator to menu item's command id.
   accelerator_util::AcceleratorTable accelerator_table_;
 
+  std::unique_ptr<views::ViewTracker> last_focused_view_tracker_;
+
   DISALLOW_COPY_AND_ASSIGN(RootView);
 };
 
diff --git a/atom/browser/ui/views/submenu_button.cc b/atom/browser/ui/views/submenu_button.cc
index 55b95abe5e8b3..e613a59f3b791 100644
--- a/atom/browser/ui/views/submenu_button.cc
+++ b/atom/browser/ui/views/submenu_button.cc
@@ -71,6 +71,11 @@ void SubmenuButton::SetUnderlineColor(SkColor color) {
   underline_color_ = color;
 }
 
+void SubmenuButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  node_data->SetName(accessible_name());
+  node_data->role = ax::mojom::Role::kPopUpButton;
+}
+
 void SubmenuButton::PaintButtonContents(gfx::Canvas* canvas) {
   views::MenuButton::PaintButtonContents(canvas);
 
diff --git a/atom/browser/ui/views/submenu_button.h b/atom/browser/ui/views/submenu_button.h
index cdd740ff6e886..12cab85e4658a 100644
--- a/atom/browser/ui/views/submenu_button.h
+++ b/atom/browser/ui/views/submenu_button.h
@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "ui/accessibility/ax_node_data.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/controls/button/menu_button.h"
 
@@ -25,6 +26,8 @@ class SubmenuButton : public views::MenuButton {
 
   base::char16 accelerator() const { return accelerator_; }
 
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+
   // views::MenuButton:
   void PaintButtonContents(gfx::Canvas* canvas) override;
 

From b2019792dbb022e48d3f671cd133ba13b247c546 Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sat, 20 Oct 2018 02:29:58 +0200
Subject: [PATCH 02/10] fix: create a temporary widget for tray icon context
 menu

---
 atom/browser/ui/win/notify_icon.cc | 30 +++++++++++++++++++++++++++---
 atom/browser/ui/win/notify_icon.h  | 11 ++++++++++-
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/atom/browser/ui/win/notify_icon.cc b/atom/browser/ui/win/notify_icon.cc
index 9b01de8a49f6c..52081edd8a0ab 100644
--- a/atom/browser/ui/win/notify_icon.cc
+++ b/atom/browser/ui/win/notify_icon.cc
@@ -15,11 +15,16 @@
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image.h"
 #include "ui/views/controls/menu/menu_runner.h"
+#include "ui/views/widget/widget.h"
 
 namespace atom {
 
 NotifyIcon::NotifyIcon(NotifyIconHost* host, UINT id, HWND window, UINT message)
-    : host_(host), icon_id_(id), window_(window), message_id_(message) {
+    : host_(host),
+      icon_id_(id),
+      window_(window),
+      message_id_(message),
+      weak_factory_(this) {
   NOTIFYICONDATA icon_data;
   InitIconData(&icon_data);
   icon_data.uFlags |= NIF_MESSAGE;
@@ -142,10 +147,25 @@ void NotifyIcon::PopUpContextMenu(const gfx::Point& pos,
   if (pos.IsOrigin())
     rect.set_origin(display::Screen::GetScreen()->GetCursorScreenPoint());
 
+  // Create a widget for the menu, otherwise we get no keyboard events, which
+  // is required for accessibility.
+  widget_.reset(new views::Widget());
+  views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
+  params.ownership =
+      views::Widget::InitParams::Ownership::WIDGET_OWNS_NATIVE_WIDGET;
+  params.bounds = gfx::Rect(0, 0, 0, 0);
+  params.force_software_compositing = true;
+
+  widget_->Init(params);
+
+  widget_->Show();
+  widget_->Activate();
   menu_runner_.reset(new views::MenuRunner(
       menu_model != nullptr ? menu_model : menu_model_,
-      views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS));
-  menu_runner_->RunMenuAt(NULL, NULL, rect, views::MENU_ANCHOR_TOPLEFT,
+      views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS,
+      base::Bind(&NotifyIcon::OnContextMenuClosed,
+                 weak_factory_.GetWeakPtr())));
+  menu_runner_->RunMenuAt(widget_.get(), NULL, rect, views::MENU_ANCHOR_TOPLEFT,
                           ui::MENU_SOURCE_MOUSE);
 }
 
@@ -172,4 +192,8 @@ void NotifyIcon::InitIconData(NOTIFYICONDATA* icon_data) {
   icon_data->uID = icon_id_;
 }
 
+void NotifyIcon::OnContextMenuClosed() {
+  widget_->Close();
+}
+
 }  // namespace atom
diff --git a/atom/browser/ui/win/notify_icon.h b/atom/browser/ui/win/notify_icon.h
index 12c8bd67bd524..79be5078ede07 100644
--- a/atom/browser/ui/win/notify_icon.h
+++ b/atom/browser/ui/win/notify_icon.h
@@ -15,6 +15,7 @@
 #include "atom/browser/ui/tray_icon.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/win/scoped_gdi_object.h"
 
 namespace gfx {
@@ -23,7 +24,8 @@ class Point;
 
 namespace views {
 class MenuRunner;
-}
+class Widget;
+}  // namespace views
 
 namespace atom {
 
@@ -63,6 +65,7 @@ class NotifyIcon : public TrayIcon {
 
  private:
   void InitIconData(NOTIFYICONDATA* icon_data);
+  void OnContextMenuClosed();
 
   // The tray that owns us.  Weak.
   NotifyIconHost* host_;
@@ -85,6 +88,12 @@ class NotifyIcon : public TrayIcon {
   // Context menu associated with this icon (if any).
   std::unique_ptr<views::MenuRunner> menu_runner_;
 
+  // Temporary widget for the context menu, needed for keyboard event capture.
+  std::unique_ptr<views::Widget> widget_;
+
+  // WeakPtrFactory for CloseClosure safety.
+  base::WeakPtrFactory<NotifyIcon> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NotifyIcon);
 };
 

From 6d7609d8ad01346753e07cc3fb650c578e22eaf1 Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sat, 20 Oct 2018 02:50:58 +0200
Subject: [PATCH 03/10] fix: focus menu bar with Alt when autohide is off

---
 atom/browser/ui/views/root_view.cc | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc
index e6c48048ee0d1..9571e682f3f17 100644
--- a/atom/browser/ui/views/root_view.cc
+++ b/atom/browser/ui/views/root_view.cc
@@ -130,9 +130,6 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) {
     return;
   }
 
-  if (!menu_bar_autohide_)
-    return;
-
   // Toggle the menu bar only when a single Alt is released.
   if (event.GetType() == blink::WebInputEvent::kRawKeyDown && IsAltKey(event)) {
     // When a single Alt is pressed:
@@ -141,7 +138,8 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) {
              IsAltKey(event) && menu_bar_alt_pressed_) {
     // When a single Alt is released right after a Alt is pressed:
     menu_bar_alt_pressed_ = false;
-    SetMenuBarVisibility(!menu_bar_visible_);
+    if (menu_bar_autohide_)
+      SetMenuBarVisibility(!menu_bar_visible_);
     View* focused_view = GetFocusManager()->GetFocusedView();
     last_focused_view_tracker_->SetView(focused_view);
     menu_bar_->RequestFocus();

From 4f7ca6e1a99bb31bfbba4df764f9b6e3fcdc960a Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sat, 20 Oct 2018 19:17:26 +0200
Subject: [PATCH 04/10] fix: make menu bar focus work more like the native
 menus

---
 atom/browser/ui/views/menu_bar.cc       |  71 +++++++++-
 atom/browser/ui/views/menu_bar.h        |   5 +
 atom/browser/ui/views/menu_delegate.cc  |   3 +
 atom/browser/ui/views/menu_delegate.h   |   1 +
 atom/browser/ui/views/root_view.cc      |  11 +-
 atom/browser/ui/views/submenu_button.cc |  11 +-
 atom/common/keyboard_util.cc            | 178 ++++++++++++------------
 atom/common/keyboard_util.h             |   5 +
 8 files changed, 185 insertions(+), 100 deletions(-)

diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc
index 6e598cb710299..207054c786b63 100644
--- a/atom/browser/ui/views/menu_bar.cc
+++ b/atom/browser/ui/views/menu_bar.cc
@@ -5,9 +5,11 @@
 #include "atom/browser/ui/views/menu_bar.h"
 
 #include <memory>
+#include <sstream>
 #include <string>
 
 #include "atom/browser/ui/views/submenu_button.h"
+#include "atom/common/keyboard_util.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/views/background.h"
 #include "ui/views/layout/box_layout.h"
@@ -99,12 +101,17 @@ void MenuBar::OnBeforeExecuteCommand() {
   window_->RestoreFocus();
 }
 
+void MenuBar::OnMenuClosed() {
+  SetAcceleratorVisibility(true);
+}
+
 bool MenuBar::AcceleratorPressed(const ui::Accelerator& accelerator) {
   views::View* focused_view = GetFocusManager()->GetFocusedView();
   if (!ContainsForFocusSearch(this, focused_view))
     return false;
 
   switch (accelerator.key_code()) {
+    case ui::VKEY_MENU:
     case ui::VKEY_ESCAPE: {
       RemovePaneFocus();
       window_->RestoreFocus();
@@ -124,9 +131,71 @@ bool MenuBar::AcceleratorPressed(const ui::Accelerator& accelerator) {
       GetFocusManager()->SetFocusedViewWithReason(
           GetLastFocusableChild(), views::FocusManager::kReasonFocusTraversal);
       return true;
-    default:
+    default: {
+      auto children = GetChildrenInZOrder();
+      for (int i = 0, n = children.size(); i < n; ++i) {
+        auto* button = static_cast<SubmenuButton*>(children[i]);
+        bool shifted = false;
+        auto keycode =
+            atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted);
+
+        if (keycode == accelerator.key_code()) {
+          const gfx::Point p(0, 0);
+          auto event = accelerator.ToKeyEvent();
+          OnMenuButtonClicked(button, p, &event);
+          return true;
+        }
+      }
+
       return false;
+    }
+  }
+}
+
+bool MenuBar::SetPaneFocus(views::View* initial_focus) {
+  bool result = views::AccessiblePaneView::SetPaneFocus(initial_focus);
+
+  if (result) {
+    auto children = GetChildrenInZOrder();
+    for (int i = 0, n = children.size(); i < n; ++i) {
+      auto* button = static_cast<SubmenuButton*>(children[i]);
+      bool shifted = false;
+      auto keycode =
+          atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted);
+
+      // We want the menu items to activate if the user presses the accelerator
+      // key, even without alt, since we are now focused on the menu bar
+      focus_manager()->RegisterAccelerator(
+          ui::Accelerator(keycode, ui::EF_NONE),
+          ui::AcceleratorManager::kNormalPriority, this);
+    }
+
+    // We want to remove focus / hide menu bar when alt is pressed again
+    focus_manager()->RegisterAccelerator(
+        ui::Accelerator(ui::VKEY_MENU, ui::EF_ALT_DOWN),
+        ui::AcceleratorManager::kNormalPriority, this);
   }
+
+  return result;
+}
+
+void MenuBar::RemovePaneFocus() {
+  views::AccessiblePaneView::RemovePaneFocus();
+  SetAcceleratorVisibility(false);
+
+  auto children = GetChildrenInZOrder();
+  for (int i = 0, n = children.size(); i < n; ++i) {
+    auto* button = static_cast<SubmenuButton*>(children[i]);
+    bool shifted = false;
+    auto keycode =
+        atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted);
+
+    focus_manager()->UnregisterAccelerator(
+        ui::Accelerator(keycode, ui::EF_NONE), this);
+  }
+
+  focus_manager()->UnregisterAccelerator(
+      ui::Accelerator(ui::VKEY_MENU, ui::EF_ALT_DOWN), this);
 }
 
 const char* MenuBar::GetClassName() const {
diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h
index 5f30b91366838..ba784ca2318af 100644
--- a/atom/browser/ui/views/menu_bar.h
+++ b/atom/browser/ui/views/menu_bar.h
@@ -48,9 +48,14 @@ class MenuBar : public views::AccessiblePaneView,
                                     AtomMenuModel** menu_model,
                                     views::MenuButton** button);
 
+  // atom::MenuDelegate::Observer
   void OnBeforeExecuteCommand() override;
+  void OnMenuClosed() override;
 
+  // views::AccessiblePaneView
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
+  bool SetPaneFocus(views::View* initial_focus) override;
+  void RemovePaneFocus() override;
 
  protected:
   // views::View:
diff --git a/atom/browser/ui/views/menu_delegate.cc b/atom/browser/ui/views/menu_delegate.cc
index 9af27504d5de8..8c24a82af5b54 100644
--- a/atom/browser/ui/views/menu_delegate.cc
+++ b/atom/browser/ui/views/menu_delegate.cc
@@ -99,6 +99,9 @@ void MenuDelegate::WillHideMenu(views::MenuItemView* menu) {
 }
 
 void MenuDelegate::OnMenuClosed(views::MenuItemView* menu) {
+  for (Observer& obs : observers_)
+    obs.OnMenuClosed();
+
   // Only switch to new menu when current menu is closed.
   if (button_to_open_)
     button_to_open_->Activate(nullptr);
diff --git a/atom/browser/ui/views/menu_delegate.h b/atom/browser/ui/views/menu_delegate.h
index efa38c6a742e8..e623f763b2eca 100644
--- a/atom/browser/ui/views/menu_delegate.h
+++ b/atom/browser/ui/views/menu_delegate.h
@@ -31,6 +31,7 @@ class MenuDelegate : public views::MenuDelegate {
   class Observer {
    public:
     virtual void OnBeforeExecuteCommand() = 0;
+    virtual void OnMenuClosed() = 0;
   };
 
   void AddObserver(Observer* obs) { observers_.AddObserver(obs); }
diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc
index 9571e682f3f17..c8a59cceb5a69 100644
--- a/atom/browser/ui/views/root_view.cc
+++ b/atom/browser/ui/views/root_view.cc
@@ -91,10 +91,6 @@ void RootView::SetMenuBarVisibility(bool visible) {
   if (!window_->content_view() || !menu_bar_ || menu_bar_visible_ == visible)
     return;
 
-  // Always show the accelerator when the auto-hide menu bar shows.
-  if (menu_bar_autohide_)
-    menu_bar_->SetAcceleratorVisibility(visible);
-
   menu_bar_visible_ = visible;
   if (visible) {
     DCHECK_EQ(child_count(), 1);
@@ -126,6 +122,10 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) {
     if (!menu_bar_visible_ &&
         (menu_bar_->HasAccelerator(event.windows_key_code)))
       SetMenuBarVisibility(true);
+
+    View* focused_view = GetFocusManager()->GetFocusedView();
+    last_focused_view_tracker_->SetView(focused_view);
+    menu_bar_->RequestFocus();
     menu_bar_->ActivateAccelerator(event.windows_key_code);
     return;
   }
@@ -140,9 +140,12 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) {
     menu_bar_alt_pressed_ = false;
     if (menu_bar_autohide_)
       SetMenuBarVisibility(!menu_bar_visible_);
+
     View* focused_view = GetFocusManager()->GetFocusedView();
     last_focused_view_tracker_->SetView(focused_view);
     menu_bar_->RequestFocus();
+    // Show accelerators when menu bar is focused
+    menu_bar_->SetAcceleratorVisibility(true);
   } else {
     // When any other keys except single Alt have been pressed/released:
     menu_bar_alt_pressed_ = false;
diff --git a/atom/browser/ui/views/submenu_button.cc b/atom/browser/ui/views/submenu_button.cc
index e613a59f3b791..b6c0c3229fe4f 100644
--- a/atom/browser/ui/views/submenu_button.cc
+++ b/atom/browser/ui/views/submenu_button.cc
@@ -80,11 +80,12 @@ void SubmenuButton::PaintButtonContents(gfx::Canvas* canvas) {
   views::MenuButton::PaintButtonContents(canvas);
 
   if (show_underline_ && (underline_start_ != underline_end_)) {
-    int padding = (width() - text_width_) / 2;
-    int underline_height = (height() + text_height_) / 2 - 2;
-    canvas->DrawLine(gfx::Point(underline_start_ + padding, underline_height),
-                     gfx::Point(underline_end_ + padding, underline_height),
-                     underline_color_);
+    float padding = (width() - text_width_) / 2;
+    float underline_height = (height() + text_height_) / 2 - 2;
+    canvas->DrawSharpLine(
+        gfx::PointF(underline_start_ + padding, underline_height),
+        gfx::PointF(underline_end_ + padding, underline_height),
+        underline_color_);
   }
 }
 
diff --git a/atom/common/keyboard_util.cc b/atom/common/keyboard_util.cc
index 1f42ad735a01d..354cabcd8d3b8 100644
--- a/atom/common/keyboard_util.cc
+++ b/atom/common/keyboard_util.cc
@@ -14,8 +14,94 @@ namespace atom {
 
 namespace {
 
-// Return key code of the char, and also determine whether the SHIFT key is
-// pressed.
+// Return key code represented by |str|.
+ui::KeyboardCode KeyboardCodeFromKeyIdentifier(const std::string& s,
+                                               bool* shifted) {
+  std::string str = base::ToLowerASCII(s);
+  if (str == "ctrl" || str == "control") {
+    return ui::VKEY_CONTROL;
+  } else if (str == "super" || str == "cmd" || str == "command" ||
+             str == "meta") {
+    return ui::VKEY_COMMAND;
+  } else if (str == "commandorcontrol" || str == "cmdorctrl") {
+#if defined(OS_MACOSX)
+    return ui::VKEY_COMMAND;
+#else
+    return ui::VKEY_CONTROL;
+#endif
+  } else if (str == "alt" || str == "option") {
+    return ui::VKEY_MENU;
+  } else if (str == "shift") {
+    return ui::VKEY_SHIFT;
+  } else if (str == "altgr") {
+    return ui::VKEY_ALTGR;
+  } else if (str == "plus") {
+    *shifted = true;
+    return ui::VKEY_OEM_PLUS;
+  } else if (str == "tab") {
+    return ui::VKEY_TAB;
+  } else if (str == "space") {
+    return ui::VKEY_SPACE;
+  } else if (str == "backspace") {
+    return ui::VKEY_BACK;
+  } else if (str == "delete") {
+    return ui::VKEY_DELETE;
+  } else if (str == "insert") {
+    return ui::VKEY_INSERT;
+  } else if (str == "enter" || str == "return") {
+    return ui::VKEY_RETURN;
+  } else if (str == "up") {
+    return ui::VKEY_UP;
+  } else if (str == "down") {
+    return ui::VKEY_DOWN;
+  } else if (str == "left") {
+    return ui::VKEY_LEFT;
+  } else if (str == "right") {
+    return ui::VKEY_RIGHT;
+  } else if (str == "home") {
+    return ui::VKEY_HOME;
+  } else if (str == "end") {
+    return ui::VKEY_END;
+  } else if (str == "pageup") {
+    return ui::VKEY_PRIOR;
+  } else if (str == "pagedown") {
+    return ui::VKEY_NEXT;
+  } else if (str == "esc" || str == "escape") {
+    return ui::VKEY_ESCAPE;
+  } else if (str == "volumemute") {
+    return ui::VKEY_VOLUME_MUTE;
+  } else if (str == "volumeup") {
+    return ui::VKEY_VOLUME_UP;
+  } else if (str == "volumedown") {
+    return ui::VKEY_VOLUME_DOWN;
+  } else if (str == "medianexttrack") {
+    return ui::VKEY_MEDIA_NEXT_TRACK;
+  } else if (str == "mediaprevioustrack") {
+    return ui::VKEY_MEDIA_PREV_TRACK;
+  } else if (str == "mediastop") {
+    return ui::VKEY_MEDIA_STOP;
+  } else if (str == "mediaplaypause") {
+    return ui::VKEY_MEDIA_PLAY_PAUSE;
+  } else if (str == "printscreen") {
+    return ui::VKEY_SNAPSHOT;
+  } else if (str.size() > 1 && str[0] == 'f') {
+    // F1 - F24.
+    int n;
+    if (base::StringToInt(str.c_str() + 1, &n) && n > 0 && n < 25) {
+      return static_cast<ui::KeyboardCode>(ui::VKEY_F1 + n - 1);
+    } else {
+      LOG(WARNING) << str << "is not available on keyboard";
+      return ui::VKEY_UNKNOWN;
+    }
+  } else {
+    if (str.size() > 2)
+      LOG(WARNING) << "Invalid accelerator token: " << str;
+    return ui::VKEY_UNKNOWN;
+  }
+}
+
+}  // namespace
+
 ui::KeyboardCode KeyboardCodeFromCharCode(base::char16 c, bool* shifted) {
   c = base::ToLowerASCII(c);
   *shifted = false;
@@ -198,94 +284,6 @@ ui::KeyboardCode KeyboardCodeFromCharCode(base::char16 c, bool* shifted) {
   }
 }
 
-// Return key code represented by |str|.
-ui::KeyboardCode KeyboardCodeFromKeyIdentifier(const std::string& s,
-                                               bool* shifted) {
-  std::string str = base::ToLowerASCII(s);
-  if (str == "ctrl" || str == "control") {
-    return ui::VKEY_CONTROL;
-  } else if (str == "super" || str == "cmd" || str == "command" ||
-             str == "meta") {
-    return ui::VKEY_COMMAND;
-  } else if (str == "commandorcontrol" || str == "cmdorctrl") {
-#if defined(OS_MACOSX)
-    return ui::VKEY_COMMAND;
-#else
-    return ui::VKEY_CONTROL;
-#endif
-  } else if (str == "alt" || str == "option") {
-    return ui::VKEY_MENU;
-  } else if (str == "shift") {
-    return ui::VKEY_SHIFT;
-  } else if (str == "altgr") {
-    return ui::VKEY_ALTGR;
-  } else if (str == "plus") {
-    *shifted = true;
-    return ui::VKEY_OEM_PLUS;
-  } else if (str == "tab") {
-    return ui::VKEY_TAB;
-  } else if (str == "space") {
-    return ui::VKEY_SPACE;
-  } else if (str == "backspace") {
-    return ui::VKEY_BACK;
-  } else if (str == "delete") {
-    return ui::VKEY_DELETE;
-  } else if (str == "insert") {
-    return ui::VKEY_INSERT;
-  } else if (str == "enter" || str == "return") {
-    return ui::VKEY_RETURN;
-  } else if (str == "up") {
-    return ui::VKEY_UP;
-  } else if (str == "down") {
-    return ui::VKEY_DOWN;
-  } else if (str == "left") {
-    return ui::VKEY_LEFT;
-  } else if (str == "right") {
-    return ui::VKEY_RIGHT;
-  } else if (str == "home") {
-    return ui::VKEY_HOME;
-  } else if (str == "end") {
-    return ui::VKEY_END;
-  } else if (str == "pageup") {
-    return ui::VKEY_PRIOR;
-  } else if (str == "pagedown") {
-    return ui::VKEY_NEXT;
-  } else if (str == "esc" || str == "escape") {
-    return ui::VKEY_ESCAPE;
-  } else if (str == "volumemute") {
-    return ui::VKEY_VOLUME_MUTE;
-  } else if (str == "volumeup") {
-    return ui::VKEY_VOLUME_UP;
-  } else if (str == "volumedown") {
-    return ui::VKEY_VOLUME_DOWN;
-  } else if (str == "medianexttrack") {
-    return ui::VKEY_MEDIA_NEXT_TRACK;
-  } else if (str == "mediaprevioustrack") {
-    return ui::VKEY_MEDIA_PREV_TRACK;
-  } else if (str == "mediastop") {
-    return ui::VKEY_MEDIA_STOP;
-  } else if (str == "mediaplaypause") {
-    return ui::VKEY_MEDIA_PLAY_PAUSE;
-  } else if (str == "printscreen") {
-    return ui::VKEY_SNAPSHOT;
-  } else if (str.size() > 1 && str[0] == 'f') {
-    // F1 - F24.
-    int n;
-    if (base::StringToInt(str.c_str() + 1, &n) && n > 0 && n < 25) {
-      return static_cast<ui::KeyboardCode>(ui::VKEY_F1 + n - 1);
-    } else {
-      LOG(WARNING) << str << "is not available on keyboard";
-      return ui::VKEY_UNKNOWN;
-    }
-  } else {
-    if (str.size() > 2)
-      LOG(WARNING) << "Invalid accelerator token: " << str;
-    return ui::VKEY_UNKNOWN;
-  }
-}
-
-}  // namespace
-
 ui::KeyboardCode KeyboardCodeFromStr(const std::string& str, bool* shifted) {
   if (str.size() == 1)
     return KeyboardCodeFromCharCode(str[0], shifted);
diff --git a/atom/common/keyboard_util.h b/atom/common/keyboard_util.h
index 651cf6a92024c..ad6dd9c0de6cb 100644
--- a/atom/common/keyboard_util.h
+++ b/atom/common/keyboard_util.h
@@ -7,10 +7,15 @@
 
 #include <string>
 
+#include "base/strings/string16.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 
 namespace atom {
 
+// Return key code of the char, and also determine whether the SHIFT key is
+// pressed.
+ui::KeyboardCode KeyboardCodeFromCharCode(base::char16 c, bool* shifted);
+
 // Return key code of the |str|, and also determine whether the SHIFT key is
 // pressed.
 ui::KeyboardCode KeyboardCodeFromStr(const std::string& str, bool* shifted);

From b78ec46d01da68a18c2c68e423ec9a8d64212a15 Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sat, 20 Oct 2018 22:34:15 +0200
Subject: [PATCH 05/10] fix: only focus menu bar if it's not already focused

---
 atom/browser/ui/views/menu_bar.cc | 6 ++++++
 atom/browser/ui/views/menu_bar.h  | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc
index 207054c786b63..1b768b1757f44 100644
--- a/atom/browser/ui/views/menu_bar.cc
+++ b/atom/browser/ui/views/menu_bar.cc
@@ -202,6 +202,12 @@ const char* MenuBar::GetClassName() const {
   return kViewClassName;
 }
 
+void MenuBar::RequestFocus() {
+  if (!HasFocus()) {
+    views::AccessiblePaneView::RequestFocus();
+  }
+}
+
 void MenuBar::OnMenuButtonClicked(views::MenuButton* source,
                                   const gfx::Point& point,
                                   const ui::Event* event) {
diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h
index ba784ca2318af..282bae423ff9b 100644
--- a/atom/browser/ui/views/menu_bar.h
+++ b/atom/browser/ui/views/menu_bar.h
@@ -48,15 +48,18 @@ class MenuBar : public views::AccessiblePaneView,
                                     AtomMenuModel** menu_model,
                                     views::MenuButton** button);
 
-  // atom::MenuDelegate::Observer
+  // atom::MenuDelegate::Observer:
   void OnBeforeExecuteCommand() override;
   void OnMenuClosed() override;
 
-  // views::AccessiblePaneView
+  // views::AccessiblePaneView:
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
   bool SetPaneFocus(views::View* initial_focus) override;
   void RemovePaneFocus() override;
 
+  // views::View:
+  void RequestFocus() override;
+
  protected:
   // views::View:
   const char* GetClassName() const override;

From bc585f266a04ad7826e7d325a36da4bfdb61697c Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sun, 21 Oct 2018 00:50:13 +0200
Subject: [PATCH 06/10] fix: track accelerator registration to avoid duplicates

---
 atom/browser/ui/views/menu_bar.cc | 25 ++++++++++++++-----------
 atom/browser/ui/views/menu_bar.h  |  3 ---
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc
index 1b768b1757f44..af7658d9b63e1 100644
--- a/atom/browser/ui/views/menu_bar.cc
+++ b/atom/browser/ui/views/menu_bar.cc
@@ -5,6 +5,7 @@
 #include "atom/browser/ui/views/menu_bar.h"
 
 #include <memory>
+#include <set>
 #include <sstream>
 #include <string>
 
@@ -157,6 +158,7 @@ bool MenuBar::SetPaneFocus(views::View* initial_focus) {
 
   if (result) {
     auto children = GetChildrenInZOrder();
+    std::set<ui::KeyboardCode> reg;
     for (int i = 0, n = children.size(); i < n; ++i) {
       auto* button = static_cast<SubmenuButton*>(children[i]);
       bool shifted = false;
@@ -165,9 +167,12 @@ bool MenuBar::SetPaneFocus(views::View* initial_focus) {
 
       // We want the menu items to activate if the user presses the accelerator
       // key, even without alt, since we are now focused on the menu bar
-      focus_manager()->RegisterAccelerator(
-          ui::Accelerator(keycode, ui::EF_NONE),
-          ui::AcceleratorManager::kNormalPriority, this);
+      if (keycode != ui::VKEY_UNKNOWN && reg.find(keycode) != reg.end()) {
+        reg.insert(keycode);
+        focus_manager()->RegisterAccelerator(
+            ui::Accelerator(keycode, ui::EF_NONE),
+            ui::AcceleratorManager::kNormalPriority, this);
+      }
     }
 
     // We want to remove focus / hide menu bar when alt is pressed again
@@ -184,14 +189,18 @@ void MenuBar::RemovePaneFocus() {
   SetAcceleratorVisibility(false);
 
   auto children = GetChildrenInZOrder();
+  std::set<ui::KeyboardCode> unreg;
   for (int i = 0, n = children.size(); i < n; ++i) {
     auto* button = static_cast<SubmenuButton*>(children[i]);
     bool shifted = false;
     auto keycode =
         atom::KeyboardCodeFromCharCode(button->accelerator(), &shifted);
 
-    focus_manager()->UnregisterAccelerator(
-        ui::Accelerator(keycode, ui::EF_NONE), this);
+    if (keycode != ui::VKEY_UNKNOWN && unreg.find(keycode) != unreg.end()) {
+      unreg.insert(keycode);
+      focus_manager()->UnregisterAccelerator(
+          ui::Accelerator(keycode, ui::EF_NONE), this);
+    }
   }
 
   focus_manager()->UnregisterAccelerator(
@@ -202,12 +211,6 @@ const char* MenuBar::GetClassName() const {
   return kViewClassName;
 }
 
-void MenuBar::RequestFocus() {
-  if (!HasFocus()) {
-    views::AccessiblePaneView::RequestFocus();
-  }
-}
-
 void MenuBar::OnMenuButtonClicked(views::MenuButton* source,
                                   const gfx::Point& point,
                                   const ui::Event* event) {
diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h
index 282bae423ff9b..a288404a84454 100644
--- a/atom/browser/ui/views/menu_bar.h
+++ b/atom/browser/ui/views/menu_bar.h
@@ -57,9 +57,6 @@ class MenuBar : public views::AccessiblePaneView,
   bool SetPaneFocus(views::View* initial_focus) override;
   void RemovePaneFocus() override;
 
-  // views::View:
-  void RequestFocus() override;
-
  protected:
   // views::View:
   const char* GetClassName() const override;

From 0a3bb340864cff2d90e2320e64ac6d7e3a66495e Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Sun, 21 Oct 2018 00:58:55 +0200
Subject: [PATCH 07/10] docs: add docs for & notation in app menu item names

---
 docs/api/menu.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/docs/api/menu.md b/docs/api/menu.md
index 21f84c66fa65f..6517836ce5008 100644
--- a/docs/api/menu.md
+++ b/docs/api/menu.md
@@ -19,6 +19,12 @@ The `menu` class has the following static methods:
 Sets `menu` as the application menu on macOS. On Windows and Linux, the
 `menu` will be set as each window's top menu.
 
+Also on Windows and Linux, you can use a `&` in the top-level item name to
+indicate which letter should get a generated accelerator. For example, using
+`&File` for the file menu would result in a generated `Alt-F` accelerator that
+opens the associated menu. The indicated character in the button label gets an
+underline. The `&` character is not displayed on the button label.
+
 Passing `null` will remove the menu bar on Windows and Linux but has no
 effect on macOS.
 

From e1a44fabc7f0c4707bc2c791c01b44dff1b340ff Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Mon, 22 Oct 2018 00:20:28 +0200
Subject: [PATCH 08/10] fix: only try to activate accelerator if it's
 registered

---
 atom/browser/ui/views/root_view.cc | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc
index c8a59cceb5a69..db03674d2e515 100644
--- a/atom/browser/ui/views/root_view.cc
+++ b/atom/browser/ui/views/root_view.cc
@@ -119,14 +119,17 @@ void RootView::HandleKeyEvent(const content::NativeWebKeyboardEvent& event) {
   // Show the submenu when "Alt+Key" is pressed.
   if (event.GetType() == blink::WebInputEvent::kRawKeyDown &&
       !IsAltKey(event) && IsAltModifier(event)) {
-    if (!menu_bar_visible_ &&
-        (menu_bar_->HasAccelerator(event.windows_key_code)))
-      SetMenuBarVisibility(true);
+    if (menu_bar_->HasAccelerator(event.windows_key_code)) {
+      if (!menu_bar_visible_) {
+        SetMenuBarVisibility(true);
 
-    View* focused_view = GetFocusManager()->GetFocusedView();
-    last_focused_view_tracker_->SetView(focused_view);
-    menu_bar_->RequestFocus();
-    menu_bar_->ActivateAccelerator(event.windows_key_code);
+        View* focused_view = GetFocusManager()->GetFocusedView();
+        last_focused_view_tracker_->SetView(focused_view);
+        menu_bar_->RequestFocus();
+      }
+
+      menu_bar_->ActivateAccelerator(event.windows_key_code);
+    }
     return;
   }
 

From 5c729d2092692041a86e0cc6dc0a5fa652444b35 Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Fri, 26 Oct 2018 04:34:38 +0200
Subject: [PATCH 09/10] fix: add friend to monitor window focus change

---
 atom/browser/ui/views/menu_bar.cc | 35 ++++++++++++++++++++-----------
 atom/browser/ui/views/menu_bar.h  | 21 ++++++++++++++++---
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc
index af7658d9b63e1..39e1614dc5ce9 100644
--- a/atom/browser/ui/views/menu_bar.cc
+++ b/atom/browser/ui/views/menu_bar.cc
@@ -35,16 +35,37 @@ const SkColor kDefaultColor = SkColorSetARGB(255, 233, 233, 233);
 
 const char MenuBar::kViewClassName[] = "ElectronMenuBar";
 
+MenuBarColorUpdater::MenuBarColorUpdater(MenuBar* menu_bar)
+    : menu_bar_(menu_bar) {}
+
+MenuBarColorUpdater::~MenuBarColorUpdater() {}
+
+void MenuBarColorUpdater::OnDidChangeFocus(views::View* focused_before,
+                                           views::View* focused_now) {
+  if (menu_bar_) {
+    // if we've changed window focus, update menu bar colors
+    const auto had_focus = menu_bar_->has_focus_;
+    menu_bar_->has_focus_ = focused_now != nullptr;
+    if (menu_bar_->has_focus_ != had_focus)
+      menu_bar_->UpdateViewColors();
+  }
+}
+
 MenuBar::MenuBar(RootView* window)
-    : background_color_(kDefaultColor), window_(window) {
+    : background_color_(kDefaultColor),
+      window_(window),
+      color_updater_(new MenuBarColorUpdater(this)) {
   RefreshColorCache();
   UpdateViewColors();
   SetFocusBehavior(FocusBehavior::ALWAYS);
   SetLayoutManager(
       std::make_unique<views::BoxLayout>(views::BoxLayout::kHorizontal));
+  window_->GetFocusManager()->AddFocusChangeListener(color_updater_.get());
 }
 
-MenuBar::~MenuBar() {}
+MenuBar::~MenuBar() {
+  window_->GetFocusManager()->RemoveFocusChangeListener(color_updater_.get());
+}
 
 void MenuBar::SetMenu(AtomMenuModel* model) {
   menu_model_ = model;
@@ -269,16 +290,6 @@ void MenuBar::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   UpdateViewColors();
 }
 
-void MenuBar::OnDidChangeFocus(View* focused_before, View* focused_now) {
-  views::AccessiblePaneView::OnDidChangeFocus(focused_before, focused_now);
-
-  // if we've changed focus, update our view
-  const auto had_focus = has_focus_;
-  has_focus_ = focused_now != nullptr;
-  if (has_focus_ != had_focus)
-    UpdateViewColors();
-}
-
 void MenuBar::RebuildChildren() {
   RemoveAllChildViews(true);
   for (int i = 0, n = GetItemCount(); i < n; ++i) {
diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h
index a288404a84454..8c44903fbb80a 100644
--- a/atom/browser/ui/views/menu_bar.h
+++ b/atom/browser/ui/views/menu_bar.h
@@ -19,6 +19,20 @@ class MenuButton;
 
 namespace atom {
 
+class MenuBarColorUpdater : public views::FocusChangeListener {
+ public:
+  explicit MenuBarColorUpdater(MenuBar* menu_bar);
+  ~MenuBarColorUpdater() override;
+
+  void OnDidChangeFocus(views::View* focused_before,
+                        views::View* focused_now) override;
+  void OnWillChangeFocus(views::View* focused_before,
+                         views::View* focused_now) override {}
+
+ private:
+  MenuBar* menu_bar_;
+};
+
 class MenuBar : public views::AccessiblePaneView,
                 public views::MenuButtonListener,
                 public atom::MenuDelegate::Observer {
@@ -67,10 +81,9 @@ class MenuBar : public views::AccessiblePaneView,
                            const ui::Event* event) override;
   void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
 
-  // views::FocusChangeListener:
-  void OnDidChangeFocus(View* focused_before, View* focused_now) override;
-
  private:
+  friend class MenuBarColorUpdater;
+
   void RebuildChildren();
   void UpdateViewColors();
 
@@ -88,6 +101,8 @@ class MenuBar : public views::AccessiblePaneView,
 
   bool has_focus_ = true;
 
+  std::unique_ptr<MenuBarColorUpdater> color_updater_;
+
   DISALLOW_COPY_AND_ASSIGN(MenuBar);
 };
 

From 147b5b956112d19ebed1ffd1edcbd0285a6f14f1 Mon Sep 17 00:00:00 2001
From: Heilig Benedek <benecene@gmail.com>
Date: Fri, 26 Oct 2018 05:00:18 +0200
Subject: [PATCH 10/10] style: add <memory> include

---
 atom/browser/ui/views/menu_bar.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h
index 8c44903fbb80a..a49ec58ac1413 100644
--- a/atom/browser/ui/views/menu_bar.h
+++ b/atom/browser/ui/views/menu_bar.h
@@ -5,6 +5,8 @@
 #ifndef ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_
 #define ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_
 
+#include <memory>
+
 #include "atom/browser/ui/atom_menu_model.h"
 #include "atom/browser/ui/views/menu_delegate.h"
 #include "atom/browser/ui/views/root_view.h"