Merge pull request #6480 from electron/webcontents-focus

Execute role-based menu items on focused web contents

atom/browser/api/atom_api_web_contents.cc

@@ -784,6 +784,13 @@ WebContents::Type WebContents::GetType() const {
   return type_;
 }
 
+#if !defined(OS_MACOSX)
+bool WebContents::IsFocused() const {
+  auto view = web_contents()->GetRenderWidgetHostView();
+  return view && view->HasFocus();
+}
+#endif
+
 bool WebContents::Equal(const WebContents* web_contents) const {
   return GetID() == web_contents->GetID();
 }
@@ -1413,6 +1420,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("showDefinitionForSelection",
                  &WebContents::ShowDefinitionForSelection)
       .SetMethod("capturePage", &WebContents::CapturePage)
+      .SetMethod("isFocused", &WebContents::IsFocused)
       .SetProperty("id", &WebContents::ID)
       .SetProperty("session", &WebContents::Session)
       .SetProperty("hostWebContents", &WebContents::HostWebContents)

atom/browser/api/atom_api_web_contents.h

@@ -67,6 +67,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
 
   int GetID() const;
   Type GetType() const;
+  bool IsFocused() const;
   bool Equal(const WebContents* web_contents) const;
   void LoadURL(const GURL& url, const mate::Dictionary& options);
   void DownloadURL(const GURL& url);

atom/browser/api/atom_api_web_contents_mac.mm

@@ -0,0 +1,30 @@
+// Copyright (c) 2016 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/api/atom_api_web_contents.h"
+
+@interface NSWindow
+- (BOOL)isKeyWindow;
+@end
+
+namespace atom {
+
+namespace api {
+
+bool WebContents::IsFocused() const {
+  if (GetType() != BACKGROUND_PAGE) {
+    auto window = web_contents()->GetTopLevelNativeWindow();
+    // On Mac the render widget host view does not lose focus when the window
+    // loses focus so check if the top level window is the key window.
+    if (window && ![window isKeyWindow])
+      return false;
+  }
+
+  auto view = web_contents()->GetRenderWidgetHostView();
+  return view && view->HasFocus();
+}
+
+}  // namespace api
+
+}  // namespace atom

docs/api/web-contents.md

@@ -463,6 +463,10 @@ let currentURL = win.webContents.getURL();
 
 Returns the title of the current web page.
 
+### `webContents.isFocused()`
+
+Returns a Boolean, whether the web page is focused.
+
 ### `webContents.isLoading()`
 
 Returns whether web page is still loading resources.

filenames.gypi

@@ -131,6 +131,7 @@
       'atom/browser/api/atom_api_tray.h',
       'atom/browser/api/atom_api_web_contents.cc',
       'atom/browser/api/atom_api_web_contents.h',
+      'atom/browser/api/atom_api_web_contents_mac.mm',
       'atom/browser/api/atom_api_web_request.cc',
       'atom/browser/api/atom_api_web_request.h',
       'atom/browser/api/atom_api_web_view_manager.cc',

lib/browser/api/menu-item-roles.js

@@ -115,7 +115,7 @@ exports.getDefaultAccelerator = (role) => {
   if (roles.hasOwnProperty(role)) return roles[role].accelerator
 }
 
-exports.execute = (role, focusedWindow) => {
+exports.execute = (role, focusedWindow, focusedWebContents) => {
   if (!roles.hasOwnProperty(role)) return false
   if (process.platform === 'darwin') return false
 
@@ -135,15 +135,8 @@ exports.execute = (role, focusedWindow) => {
     return true
   }
 
-  if (webContentsMethod && focusedWindow != null) {
-    const {webContents} = focusedWindow
-    if (webContents) {
-      if (webContents.isDevToolsFocused()) {
-        webContents.devToolsWebContents[webContentsMethod]()
-      } else {
-        webContents[webContentsMethod]()
-      }
-    }
+  if (webContentsMethod && focusedWebContents != null) {
+    focusedWebContents[webContentsMethod]()
     return true
   }
 

lib/browser/api/menu-item.js

@@ -48,13 +48,13 @@ const MenuItem = function (options) {
   this.overrideReadOnlyProperty('commandId', ++nextCommandId)
 
   const click = options.click
-  this.click = (event, focusedWindow) => {
+  this.click = (event, focusedWindow, focusedWebContents) => {
     // Manually flip the checked flags when clicked.
     if (this.type === 'checkbox' || this.type === 'radio') {
       this.checked = !this.checked
     }
 
-    if (!roles.execute(this.role, focusedWindow)) {
+    if (!roles.execute(this.role, focusedWindow, focusedWebContents)) {
       if (typeof click === 'function') {
         click(this, focusedWindow, event)
       } else if (typeof this.selector === 'string' && process.platform === 'darwin') {

lib/browser/api/menu.js

@@ -1,7 +1,6 @@
 'use strict'
 
-const BrowserWindow = require('electron').BrowserWindow
-const MenuItem = require('electron').MenuItem
+const {BrowserWindow, MenuItem, webContents} = require('electron')
 const EventEmitter = require('events').EventEmitter
 const v8Util = process.atomBinding('v8_util')
 const bindings = process.atomBinding('menu')
@@ -117,8 +116,9 @@ Menu.prototype._init = function () {
       return command != null ? command.icon : undefined
     },
     executeCommand: (event, commandId) => {
-      var command = this.commandsMap[commandId]
-      return command != null ? command.click(event, BrowserWindow.getFocusedWindow()) : undefined
+      const command = this.commandsMap[commandId]
+      if (command == null) return
+      command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents())
     },
     menuWillShow: () => {
       // Make sure radio groups have at least one menu item seleted.

lib/browser/api/web-contents.js

@@ -2,6 +2,7 @@
 
 const {EventEmitter} = require('events')
 const {app, ipcMain, session, Menu, NavigationController} = require('electron')
+const {getAllWebContents} = process.atomBinding('web_contents')
 
 // session is not used here, the purpose is to make sure session is initalized
 // before the webContents module.
@@ -244,5 +245,17 @@ module.exports = {
 
   fromId (id) {
     return binding.fromId(id)
+  },
+
+  getFocusedWebContents () {
+    let focused = null
+    for (let contents of getAllWebContents()) {
+      if (!contents.isFocused()) continue
+      if (focused == null) focused = contents
+      // Return webview web contents which may be embedded inside another
+      // web contents that is also reporting as focused
+      if (contents.getType() === 'webview') return contents
+    }
+    return focused
   }
 }