Fixes focus issues by:

- capturing the browser's selection changes
- converting the activeElement object to a selector
- returning focus after menu is interacted with by:
  - using document.querySelectAll() to return focus to original element
  - executing window action which actually handles the menu action

This commit also cleans up `main.js` by properly encapsulating the
handlers required into a new method, `registerCustomTitlebarHandlers`

cc: @bbondy

app/renderer/components/menubar.js

@@ -10,7 +10,7 @@ const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem
 const keyCodes = require('../../../js/constants/keyCodes')
 const { wrappingClamp } = require('../../common/lib/formatUtil')
 
-const showContextMenu = (rect, submenu, activeFrame) => {
+const showContextMenu = (rect, submenu, lastFocusedSelector) => {
   windowActions.setContextMenuDetail(Immutable.fromJS({
     left: rect.left,
     top: rect.bottom,
@@ -20,9 +20,9 @@ const showContextMenu = (rect, submenu, activeFrame) => {
       }
       submenuItem.click = function (e) {
         e.preventDefault()
-        if (activeFrame && activeFrame.has('key')) {
+        if (lastFocusedSelector) {
           // Send focus back to the active web frame
-          const results = document.querySelectorAll('webview[data-frame-key="' + activeFrame.get('key') + '"]')
+          const results = document.querySelectorAll(lastFocusedSelector)
           if (results.length === 1) {
             results[0].focus()
           }
@@ -54,7 +54,7 @@ class MenubarItem extends ImmutableComponent {
     // Otherwise, mark item as selected and show its context menu
     windowActions.setMenubarSelectedLabel(this.props.label)
     const rect = e.target.getBoundingClientRect()
-    showContextMenu(rect, this.props.submenu, this.props.activeFrame)
+    showContextMenu(rect, this.props.submenu, this.props.lastFocusedSelector)
   }
   onMouseOver (e) {
     const selected = this.props.menubar.props.selectedLabel
@@ -160,7 +160,7 @@ class Menubar extends ImmutableComponent {
           // Context menu already being displayed; auto-open the next one
           if (this.props.contextMenuDetail && this.selectedTemplate && nextRect) {
             windowActions.setSubmenuSelectedIndex(0)
-            showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS(), this.props.activeFrame)
+            showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS(), this.props.lastFocusedSelector)
           }
         }
         break
@@ -174,7 +174,7 @@ class Menubar extends ImmutableComponent {
           if (!this.props.contextMenuDetail && this.selectedRect) {
             // First time hitting up/down; popup the context menu
             windowActions.setSubmenuSelectedIndex(0)
-            showContextMenu(this.selectedRect, this.selectedTemplate.toJS(), this.props.activeFrame)
+            showContextMenu(this.selectedRect, this.selectedTemplate.toJS(), this.props.lastFocusedSelector)
           } else {
             // Context menu already visible; move selection up or down
             const nextIndex = wrappingClamp(
@@ -198,7 +198,7 @@ class Menubar extends ImmutableComponent {
           label: menubarItem.get('label'),
           submenu: menubarItem.get('submenu').toJS(),
           menubar: this,
-          activeFrame: this.props.activeFrame
+          lastFocusedSelector: this.props.lastFocusedSelector
         }
         if (props.label === this.props.selectedLabel) {
           props.selected = true

docs/state.md

@@ -374,7 +374,8 @@ WindowStore
     menubar: { // windows only
       isVisible: boolean, // true if Menubar control is visible
       selectedLabel: string, // label of menu that is selected (or null for none selected)
-      selectedIndex: number // index of the selected context menu item
+      selectedIndex: number, // index of the selected context menu item
+      lastFocusedSelector: string // selector for the last selected element (browser ui, not frame content)
     }
   },
   searchDetail: {

docs/windowActions.md

@@ -824,14 +824,54 @@ Used to track which menubar item is currently selected (or null for none selecte
 
 ### resetMenuState() 
 
-Used by main.js when click happens on content area (not on a link or react control).
+Used by `main.js` when click happens on content area (not on a link or react control).
 - closes context menu
 - closes popup menu
 - nulls out menubar item selected (Windows only)
 - hides menubar if auto-hide preference is set (Windows only)
 
 
 
+### setSubmenuSelectedIndex(index) 
+
+(Windows only)
+Used to track selected index of a context menu
+Needed because arrow keys can be used to navigate the custom menu
+
+**Parameters**
+
+**index**: `number`, zero based index of the item.
+  Index excludes menu separators and hidden items.
+
+
+
+### setLastFocusedSelector(selector) 
+
+(Windows only at the moment)
+Used to track last selected element (typically the URL bar or the frame)
+Important because focus is lost when using the custom menu and needs
+to be returned in order for the cut/copy operation to work
+
+**Parameters**
+
+**selector**: `string`, selector used w/ querySelectorAll to return focus
+  after a menu item is selected (via the custom titlebar / menubar)
+
+
+
+### gotResponseDetails(tabId, details) 
+
+Used to get response details (such as the HTTP response code) from a response
+See `eventStore.js` for an example use-case
+
+**Parameters**
+
+**tabId**: `number`, the tab id to set
+
+**details**: `Object`, object containing response details
+
+
+
 
 * * *
 

js/actions/windowActions.js

@@ -1046,7 +1046,7 @@ const windowActions = {
   },
 
   /**
-   * Used by main.js when click happens on content area (not on a link or react control).
+   * Used by `main.js` when click happens on content area (not on a link or react control).
    * - closes context menu
    * - closes popup menu
    * - nulls out menubar item selected (Windows only)
@@ -1058,13 +1058,41 @@ const windowActions = {
     })
   },
 
+  /**
+   * (Windows only)
+   * Used to track selected index of a context menu
+   * Needed because arrow keys can be used to navigate the custom menu
+   * @param {number} index - zero based index of the item.
+   *   Index excludes menu separators and hidden items.
+   */
   setSubmenuSelectedIndex: function (index) {
     dispatch({
       actionType: WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX,
       index
     })
   },
 
+  /**
+   * (Windows only at the moment)
+   * Used to track last selected element (typically the URL bar or the frame)
+   * Important because focus is lost when using the custom menu and needs
+   * to be returned in order for the cut/copy operation to work
+   * @param {string} selector - selector used w/ querySelectorAll to return focus
+   *   after a menu item is selected (via the custom titlebar / menubar)
+   */
+  setLastFocusedSelector: function (selector) {
+    dispatch({
+      actionType: WindowConstants.WINDOW_SET_LAST_FOCUSED_SELECTOR,
+      selector
+    })
+  },
+
+  /**
+   * Used to get response details (such as the HTTP response code) from a response
+   * See `eventStore.js` for an example use-case
+   * @param {number} tabId - the tab id to set
+   * @param {Object} details - object containing response details
+   */
   gotResponseDetails: function (tabId, details) {
     dispatch({
       actionType: WindowConstants.WINDOW_GOT_RESPONSE_DETAILS,

js/components/main.js

@@ -121,16 +121,18 @@ class Main extends ImmutableComponent {
           break
       }
     })
-    document.addEventListener('keyup', (e) => {
-      const customTitlebar = this.customTitlebar
-      switch (e.which) {
-        case keyCodes.ALT:
-          if (customTitlebar.enabled) {
+  }
+
+  registerCustomTitlebarHandlers () {
+    if (this.customTitlebar.enabled) {
+      document.addEventListener('keyup', (e) => {
+        const customTitlebar = this.customTitlebar
+        switch (e.which) {
+          case keyCodes.ALT:
             e.preventDefault()
 
-            const defaultLabel = customTitlebar.menubarTemplate
-              ? customTitlebar.menubarTemplate.getIn([0, 'label'])
-              : null
+            const menubarTemplate = this.props.appState.getIn(['menu', 'template'])
+            const defaultLabel = menubarTemplate.getIn([0, 'label'])
 
             if (getSetting(settings.AUTO_HIDE_MENU)) {
               windowActions.toggleMenubarVisible(null, defaultLabel)
@@ -142,10 +144,8 @@ class Main extends ImmutableComponent {
                 windowActions.setMenubarSelectedLabel(defaultLabel)
               }
             }
-          }
-          break
-        case keyCodes.ESC:
-          if (customTitlebar.enabled) {
+            break
+          case keyCodes.ESC:
             if (getSetting(settings.AUTO_HIDE_MENU) && customTitlebar.menubarVisible && !customTitlebar.menubarSelectedLabel) {
               e.preventDefault()
               windowActions.toggleMenubarVisible(false)
@@ -156,10 +156,25 @@ class Main extends ImmutableComponent {
               windowActions.setMenubarSelectedLabel()
               windowActions.setContextMenuDetail()
             }
+            break
+        }
+      })
+
+      document.addEventListener('focus', (e) => {
+        let selector = document.activeElement.id
+          ? '#' + document.activeElement.id
+          : null
+
+        if (!selector && document.activeElement.tagName === 'WEBVIEW') {
+          const frameKeyAttribute = document.activeElement.getAttribute('data-frame-key')
+          if (frameKeyAttribute) {
+            selector = 'webview[data-frame-key="' + frameKeyAttribute + '"]'
           }
-          break
-      }
-    })
+        }
+
+        windowActions.setLastFocusedSelector(selector)
+      }, true)
+    }
   }
 
   exitFullScreen () {
@@ -298,6 +313,7 @@ class Main extends ImmutableComponent {
   componentDidMount () {
     this.registerSwipeListener()
     this.registerWindowLevelShortcuts()
+    this.registerCustomTitlebarHandlers()
 
     ipc.on(messages.SHORTCUT_NEW_FRAME, (event, url, options = {}) => {
       if (options.singleFrame) {
@@ -765,6 +781,7 @@ class Main extends ImmutableComponent {
       menubarTemplate: menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null,
       menubarSelectedLabel: this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']),
       menubarSelectedIndex: this.props.windowState.getIn(['ui', 'menubar', 'selectedIndex']),
+      lastFocusedSelector: this.props.windowState.getIn(['ui', 'menubar', 'lastFocusedSelector']),
       isMaximized: this.props.windowState.getIn(['ui', 'isMaximized'])
     }
   }
@@ -845,7 +862,7 @@ class Main extends ImmutableComponent {
                       selectedIndex={customTitlebar.menubarSelectedIndex}
                       contextMenuDetail={this.props.windowState.get('contextMenuDetail')}
                       autohide={getSetting(settings.AUTO_HIDE_MENU)}
-                      activeFrame={activeFrame} />
+                      lastFocusedSelector={customTitlebar.lastFocusedSelector} />
                   </div>
                   : null
               }

js/constants/windowConstants.js

@@ -74,6 +74,7 @@ const windowConstants = {
   WINDOW_SET_MENUBAR_SELECTED_LABEL: _,
   WINDOW_RESET_MENU_STATE: _,
   WINDOW_SET_SUBMENU_SELECTED_INDEX: _,
+  WINDOW_SET_LAST_FOCUSED_SELECTOR: _,
   WINDOW_GOT_RESPONSE_DETAILS: _
 }
 

js/stores/windowStore.js

@@ -821,6 +821,9 @@ const doAction = (action) => {
         ? null
         : proposedIndex)
       break
+    case WindowConstants.WINDOW_SET_LAST_FOCUSED_SELECTOR:
+      windowState = windowState.setIn(['ui', 'menubar', 'lastFocusedSelector'], action.selector)
+      break
 
     default:
   }