Bug 416771 - Allow window.focus() to switch tabs. r=NeilDeakin draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 17 May 2019 17:52:15 +0200
changeset 2008480 d0119439a8505e9d3016cc41ff3ed0a9d08e3b2c
parent 2008479 7e228ad30ff1cad8ec683353ed687893a12dab9c
child 2008481 e104cbadc98d9ac33cd44567205627dcc074a100
push id363917
push useremilio@crisal.io
push dateSat, 18 May 2019 06:31:42 +0000
treeherdertry@46e3df2f90da [default view] [failures only]
reviewersNeilDeakin
bugs416771
milestone68.0a1
Bug 416771 - Allow window.focus() to switch tabs. r=NeilDeakin I think this should just work as-is, but probably a test would be on point. Differential Revision: https://phabricator.services.mozilla.com/D31643
browser/base/content/tabbrowser.js
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/base/nsGlobalWindowOuter.cpp
dom/ipc/BrowserBridgeChild.cpp
dom/ipc/BrowserParent.cpp
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -25,16 +25,17 @@ window._gBrowser = {
     Services.obs.addObserver(this, "contextual-identity-updated");
 
     Services.els.addSystemEventListener(document, "keydown", this, false);
     if (AppConstants.platform == "macosx") {
       Services.els.addSystemEventListener(document, "keypress", this, false);
     }
     window.addEventListener("sizemodechange", this);
     window.addEventListener("occlusionstatechange", this);
+    window.addEventListener("framefocusrequested", this);
 
     this._setupInitialBrowserAndTab();
 
     if (Services.prefs.getBoolPref("browser.display.use_system_colors")) {
       this.tabpanels.style.backgroundColor = "-moz-default-background-color";
     } else if (Services.prefs.getIntPref("browser.display.document_color_use") == 2) {
       this.tabpanels.style.backgroundColor =
         Services.prefs.getCharPref("browser.display.background_color");
@@ -4195,16 +4196,27 @@ window._gBrowser = {
           gBrowser.removeMultiSelectedTabs();
         } else if (!this.selectedTab.pinned) {
           this.removeCurrentTab({ animate: true });
         }
         aEvent.preventDefault();
     }
   },
 
+  _focusBrowser(aBrowser, aCanRaiseWindow = true) {
+    let tab = this.getTabForBrowser(aBrowser);
+    if (!tab || this.selectedTab == tab)
+      return false;
+    this.selectedTab = tab;
+    if (aCanRaiseWindow) {
+      window.focus();
+    }
+    return true;
+  },
+
   _handleKeyPressEventMac(aEvent) {
     if (!aEvent.isTrusted) {
       // Don't let untrusted events mess with tabs.
       return;
     }
 
     // Skip this only if something has explicitly cancelled it.
     if (aEvent.defaultCancelled) {
@@ -4293,16 +4305,23 @@ window._gBrowser = {
   handleEvent(aEvent) {
     switch (aEvent.type) {
       case "keydown":
         this._handleKeyDownEvent(aEvent);
         break;
       case "keypress":
         this._handleKeyPressEventMac(aEvent);
         break;
+      case "framefocusrequested": {
+        let canRaiseWindow = !aEvent.cancelable;
+        if (this._focusBrowser(aEvent.target, canRaiseWindow)) {
+          aEvent.preventDefault();
+        }
+        break;
+      }
       case "sizemodechange":
       case "occlusionstatechange":
         if (aEvent.target == window && !this._switcher) {
           this.selectedBrowser.preserveLayers(
             window.windowState == window.STATE_MINIMIZED || window.isFullyOccluded);
           this.selectedBrowser.docShellIsActive = this.shouldActivateDocShell(this.selectedBrowser);
         }
         break;
@@ -4345,21 +4364,17 @@ window._gBrowser = {
       }
       case "contextmenu":
       {
         openContextMenu(aMessage);
         break;
       }
       case "DOMWindowFocus":
       {
-        let tab = this.getTabForBrowser(browser);
-        if (!tab)
-          return undefined;
-        this.selectedTab = tab;
-        window.focus();
+        this._focusBrowser(browser);
         break;
       }
       case "Browser:Init":
       {
         let tab = this.getTabForBrowser(browser);
         if (!tab)
           return undefined;
 
@@ -4503,16 +4518,17 @@ window._gBrowser = {
     }
 
     Services.els.removeSystemEventListener(document, "keydown", this, false);
     if (AppConstants.platform == "macosx") {
       Services.els.removeSystemEventListener(document, "keypress", this, false);
     }
     window.removeEventListener("sizemodechange", this);
     window.removeEventListener("occlusionstatechange", this);
+    window.removeEventListener("framefocusrequested", this);
 
     if (gMultiProcessBrowser) {
       let messageManager = window.getGroupMessageManager("browsers");
       messageManager.removeMessageListener("DOMTitleChanged", this);
       window.messageManager.removeMessageListener("contextmenu", this);
 
       if (this._switcher) {
         this._switcher.destroy();
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -4171,16 +4171,41 @@ nsresult nsContentUtils::DispatchFocusCh
   if (!doc) {
     return NS_ERROR_FAILURE;
   }
 
   return DispatchChromeEvent(doc, aWindow, NS_LITERAL_STRING("DOMWindowFocus"),
                              CanBubble::eYes, Cancelable::eYes);
 }
 
+void nsContentUtils::RequestFrameFocus(Element& aFrameElement, bool aCanRaise) {
+  RefPtr<Element> target = &aFrameElement;
+  bool defaultAction = true;
+  DispatchEventOnlyToChrome(target->OwnerDoc(), target,
+                            NS_LITERAL_STRING("framefocusrequested"),
+                            CanBubble::eYes,
+                            aCanRaise ? Cancelable::eNo : Cancelable::eYes,
+                            &defaultAction);
+  if (!defaultAction) {
+    return;
+  }
+
+  nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
+  if (!fm) {
+    return;
+  }
+
+  uint32_t flags = nsIFocusManager::FLAG_NOSCROLL;
+  if (aCanRaise) {
+    flags |= nsIFocusManager::FLAG_RAISE;
+  }
+
+  fm->SetFocus(target, flags);
+}
+
 nsresult nsContentUtils::DispatchEventOnlyToChrome(
     Document* aDoc, nsISupports* aTarget, const nsAString& aEventName,
     CanBubble aCanBubble, Cancelable aCancelable, bool* aDefaultAction) {
   return DispatchEvent(aDoc, aTarget, aEventName, aCanBubble, aCancelable,
                        Composed::eDefault, Trusted::eYes, aDefaultAction,
                        ChromeOnlyDispatch::eYes);
 }
 
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -1514,16 +1514,22 @@ class nsContentUtils {
 
   /**
    * Helper function for dispatching a "DOMWindowFocus" event to
    * the chrome event handler of the given DOM Window. This has the effect
    * of focusing the corresponding tab and bringing the browser window
    * to the foreground.
    */
   static nsresult DispatchFocusChromeEvent(nsPIDOMWindowOuter* aWindow);
+  /**
+   * Helper to dispatch a "framefocusrequested" event to chrome, which like the
+   * function above also focuses the corresponding tab. It will only bring the
+   * window to the foreground if aCanRaise is true.
+   */
+  static void RequestFrameFocus(Element& aFrameElement, bool aCanRaise);
 
   /**
    * This method creates and dispatches a trusted event.
    * If aTarget is not a chrome object, the nearest chrome object in the
    * propagation path will be used as the start of the event target chain.
    * This method is different than DispatchChromeEvent, which always dispatches
    * events to chrome event handler. DispatchEventOnlyToChrome works like
    * DispatchTrustedEvent in the case aTarget is a chrome object.
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -4857,24 +4857,17 @@ void nsGlobalWindowOuter::PromptOuter(co
 
 void nsGlobalWindowOuter::FocusOuter() {
   nsFocusManager* fm = nsFocusManager::GetFocusManager();
   if (!fm) {
     return;
   }
 
   nsCOMPtr<nsIBaseWindow> baseWin = do_QueryInterface(mDocShell);
-
-  bool isVisible = false;
-  if (baseWin) {
-    baseWin->GetVisibility(&isVisible);
-  }
-
-  if (!isVisible) {
-    // A hidden tab is being focused, ignore this call.
+  if (!baseWin) {
     return;
   }
 
   nsCOMPtr<nsPIDOMWindowInner> caller = do_QueryInterface(GetEntryGlobal());
   nsPIDOMWindowOuter* callerOuter = caller ? caller->GetOuterWindow() : nullptr;
   nsCOMPtr<nsPIDOMWindowOuter> opener = GetOpener();
 
   // Enforce dom.disable_window_flip (for non-chrome), but still allow the
@@ -4923,24 +4916,18 @@ void nsGlobalWindowOuter::FocusOuter() {
   nsCOMPtr<nsPIDOMWindowOuter> parent =
       parentDsti ? parentDsti->GetWindow() : nullptr;
   if (parent) {
     nsCOMPtr<Document> parentdoc = parent->GetDoc();
     if (!parentdoc) {
       return;
     }
 
-    RefPtr<Element> frame = parentdoc->FindContentForSubDocument(mDoc);
-    if (frame) {
-      uint32_t flags = nsIFocusManager::FLAG_NOSCROLL;
-      if (canFocus) flags |= nsIFocusManager::FLAG_RAISE;
-      DebugOnly<nsresult> rv = fm->SetFocus(frame, flags);
-      MOZ_ASSERT(NS_SUCCEEDED(rv),
-                 "SetFocus only fails if the first argument is null, "
-                 "but we pass an element");
+    if (Element* frame = parentdoc->FindContentForSubDocument(mDoc)) {
+      nsContentUtils::RequestFrameFocus(*frame, canFocus);
     }
     return;
   }
 
   if (canFocus) {
     // if there is no parent, this must be a toplevel window, so raise the
     // window if canFocus is true. If this is a child process, the raise
     // window request will get forwarded to the parent by the puppet widget.
--- a/dom/ipc/BrowserBridgeChild.cpp
+++ b/dom/ipc/BrowserBridgeChild.cpp
@@ -154,46 +154,34 @@ IPCResult BrowserBridgeChild::RecvSetLay
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult BrowserBridgeChild::RecvRequestFocus(
     const bool& aCanRaise) {
   // Adapted from BrowserParent
-  nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
-  if (!fm) {
-    return IPC_OK();
-  }
-
   RefPtr<Element> owner = mFrameLoader->GetOwnerContent();
   if (!owner) {
     return IPC_OK();
   }
-
-  uint32_t flags = nsIFocusManager::FLAG_NOSCROLL;
-  if (aCanRaise) {
-    flags |= nsIFocusManager::FLAG_RAISE;
-  }
-
-  fm->SetFocus(owner, flags);
+  nsContentUtils::RequestFrameFocus(*owner, aCanRaise);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult BrowserBridgeChild::RecvMoveFocus(
     const bool& aForward, const bool& aForDocumentNavigation) {
   // Adapted from BrowserParent
   nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
   if (!fm) {
     return IPC_OK();
   }
 
   RefPtr<Element> owner = mFrameLoader->GetOwnerContent();
-
-  if (!owner || !owner->OwnerDoc()) {
+  if (!owner) {
     return IPC_OK();
   }
 
   RefPtr<Element> dummy;
 
   uint32_t type =
       aForward
           ? (aForDocumentNavigation
--- a/dom/ipc/BrowserParent.cpp
+++ b/dom/ipc/BrowserParent.cpp
@@ -2116,36 +2116,26 @@ mozilla::ipc::IPCResult BrowserParent::R
   bool consumed = (rv == NS_SUCCESS_EVENT_CONSUMED);
   HandledWindowedPluginKeyEvent(aKeyEventData, consumed);
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult BrowserParent::RecvRequestFocus(const bool& aCanRaise) {
   LOGBROWSERFOCUS(("RecvRequestFocus %p, aCanRaise: %d", this, aCanRaise));
-  BrowserBridgeParent* bridgeParent = GetBrowserBridgeParent();
-  if (bridgeParent) {
+  if (BrowserBridgeParent* bridgeParent = GetBrowserBridgeParent()) {
     mozilla::Unused << bridgeParent->SendRequestFocus(aCanRaise);
     return IPC_OK();
   }
 
-  nsCOMPtr<nsIFocusManager> fm = nsFocusManager::GetFocusManager();
-  if (!fm) {
-    return IPC_OK();
-  }
-
   if (!mFrameElement) {
     return IPC_OK();
   }
 
-  uint32_t flags = nsIFocusManager::FLAG_NOSCROLL;
-  if (aCanRaise) flags |= nsIFocusManager::FLAG_RAISE;
-
-  RefPtr<Element> element = mFrameElement;
-  fm->SetFocus(element, flags);
+  nsContentUtils::RequestFrameFocus(*mFrameElement, aCanRaise);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult BrowserParent::RecvEnableDisableCommands(
     const nsString& aAction, nsTArray<nsCString>&& aEnabledCommands,
     nsTArray<nsCString>&& aDisabledCommands) {
   nsCOMPtr<nsIBrowser> browser =
       mFrameElement ? mFrameElement->AsBrowser() : nullptr;