Bug 1689155 - Don't necessarily show outlines on refocus. r=edgar

Preserve the last state of when we focused the element in that window,
if the focus method is unknown.

Differential Revision: https://phabricator.services.mozilla.com/D104863

dom/base/nsFocusManager.cpp

@@ -1164,12 +1164,20 @@ nsFocusManager::BlurredElementInfo::~BlurredElementInfo() = default;
 
 // https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo
 static bool ShouldMatchFocusVisible(
-    const Element& aElement, int32_t aFocusFlags,
-    const Maybe<nsFocusManager::BlurredElementInfo>& aBlurredElementInfo) {
+    nsPIDOMWindowOuter* aWindow, const Element& aElement,
+    int32_t aFocusFlags,
+    const Maybe<nsFocusManager::BlurredElementInfo>& aBlurredElementInfo,
+    bool aIsRefocus, bool aRefocusedElementUsedToShowOutline) {
   // If we were explicitly requested to show the ring, do it.
   if (aFocusFlags & nsIFocusManager::FLAG_SHOWRING) {
     return true;
   }
+
+  if (aWindow->ShouldShowFocusRing()) {
+    // The window decision also trumps any other heuristic.
+    return true;
+  }
+
   // Any element which supports keyboard input (such as an input element, or any
   // other element which may trigger a virtual keyboard to be shown on focus if
   // a physical keyboard is not present) should always match :focus-visible when
@@ -1201,6 +1209,12 @@ static bool ShouldMatchFocusVisible(
       // Conversely, if the active element does not match :focus-visible, and a
       // script causes focus to move elsewhere, the newly focused element should
       // not match :focus-visible.
+      //
+      // There's an special-case here. If this is a refocus, we just keep the
+      // outline as it was before, the focus isn't moving after all.
+      if (aIsRefocus) {
+        return aRefocusedElementUsedToShowOutline;
+      }
       return !aBlurredElementInfo || aBlurredElementInfo->mHadRing;
     case InputContextAction::CAUSE_MOUSE:
     case InputContextAction::CAUSE_TOUCH:
@@ -2452,22 +2466,24 @@ void nsFocusManager::Focus(
     mFocusedElement = aElement;
 
     nsIContent* focusedNode = aWindow->GetFocusedElement();
-    bool isRefocus = focusedNode && focusedNode == aElement;
+    const bool sendFocusEvent = aElement && aElement->IsInComposedDoc() &&
+                                !IsNonFocusableRoot(aElement);
+    const bool isRefocus = focusedNode && focusedNode == aElement;
+    const bool shouldShowFocusRing =
+        sendFocusEvent &&
+        ShouldMatchFocusVisible(
+            aWindow, *aElement, aFlags, aBlurredElementInfo, isRefocus,
+            isRefocus && aWindow->FocusedElementShowedOutline());
 
-    aWindow->SetFocusedElement(aElement, focusMethod);
+    aWindow->SetFocusedElement(aElement, focusMethod, false,
+                               shouldShowFocusRing);
 
     // if the focused element changed, scroll it into view
     if (aElement && aFocusChanged) {
       ScrollIntoView(presShell, aElement, aFlags);
     }
-
-    bool sendFocusEvent = aElement && aElement->IsInComposedDoc() &&
-                          !IsNonFocusableRoot(aElement);
     nsPresContext* presContext = presShell->GetPresContext();
     if (sendFocusEvent) {
-      const bool shouldShowFocusRing =
-          aWindow->ShouldShowFocusRing() ||
-          ShouldMatchFocusVisible(*aElement, aFlags, aBlurredElementInfo);
       NotifyFocusStateChange(aElement, nullptr, aFlags,
                              /* aGettingFocus = */ true, shouldShowFocusRing);
 

dom/base/nsGlobalWindowInner.cpp

@@ -4341,7 +4341,8 @@ void nsGlobalWindowInner::StopVRActivity() {
 
 void nsGlobalWindowInner::SetFocusedElement(Element* aElement,
                                             uint32_t aFocusMethod,
-                                            bool aNeedsFocus) {
+                                            bool aNeedsFocus,
+                                            bool aWillShowOutline) {
   if (aElement && aElement->GetComposedDoc() != mDoc) {
     NS_WARNING("Trying to set focus to a node from a wrong document");
     return;
@@ -4351,13 +4352,17 @@ void nsGlobalWindowInner::SetFocusedElement(Element* aElement,
     NS_ASSERTION(!aElement, "Trying to focus cleaned up window!");
     aElement = nullptr;
     aNeedsFocus = false;
+    aWillShowOutline = false;
   }
   if (mFocusedElement != aElement) {
     UpdateCanvasFocus(false, aElement);
     mFocusedElement = aElement;
+    // TODO: Maybe this should be set on refocus too?
     mFocusMethod = aFocusMethod & FOCUSMETHOD_MASK;
   }
 
+  mFocusedElementShowedOutlines = aWillShowOutline;
+
   if (mFocusedElement) {
     // if a node was focused by a keypress, turn on focus rings for the
     // window.
@@ -4366,7 +4371,9 @@ void nsGlobalWindowInner::SetFocusedElement(Element* aElement,
     }
   }
 
-  if (aNeedsFocus) mNeedsFocus = aNeedsFocus;
+  if (aNeedsFocus) {
+    mNeedsFocus = aNeedsFocus;
+  }
 }
 
 uint32_t nsGlobalWindowInner::GetFocusMethod() { return mFocusMethod; }

dom/base/nsGlobalWindowInner.h

@@ -1115,13 +1115,13 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
 
   bool IsInModalState();
 
-  virtual void SetFocusedElement(mozilla::dom::Element* aElement,
-                                 uint32_t aFocusMethod = 0,
-                                 bool aNeedsFocus = false) override;
+  void SetFocusedElement(mozilla::dom::Element* aElement,
+                         uint32_t aFocusMethod = 0, bool aNeedsFocus = false,
+                         bool aWillShowOutline = false) override;
 
-  virtual uint32_t GetFocusMethod() override;
+  uint32_t GetFocusMethod() override;
 
-  virtual bool ShouldShowFocusRing() override;
+  bool ShouldShowFocusRing() override;
 
   // Inner windows only.
   void UpdateCanvasFocus(bool aFocusChanged, nsIContent* aNewContent);
@@ -1397,7 +1397,7 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
   uint32_t mSerial;
 #endif
 
-  // the method that was used to focus mFocusedNode
+  // the method that was used to focus mFocusedElement
   uint32_t mFocusMethod;
 
   // The current idle request callback handle

dom/base/nsGlobalWindowOuter.cpp

@@ -6688,9 +6688,10 @@ void nsGlobalWindowOuter::SetChromeEventHandler(
 
 void nsGlobalWindowOuter::SetFocusedElement(Element* aElement,
                                             uint32_t aFocusMethod,
-                                            bool aNeedsFocus) {
-  FORWARD_TO_INNER_VOID(SetFocusedElement,
-                        (aElement, aFocusMethod, aNeedsFocus));
+                                            bool aNeedsFocus,
+                                            bool aWillShowOutline) {
+  FORWARD_TO_INNER_VOID(SetFocusedElement, (aElement, aFocusMethod, aNeedsFocus,
+                                            aWillShowOutline));
 }
 
 uint32_t nsGlobalWindowOuter::GetFocusMethod() {

dom/base/nsGlobalWindowOuter.h

@@ -895,26 +895,25 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget,
   nsIntSize CSSToDevIntPixelsForBaseWindow(nsIntSize aCSSSize,
                                            nsIBaseWindow* aWindow);
 
-  virtual void SetFocusedElement(mozilla::dom::Element* aElement,
-                                 uint32_t aFocusMethod = 0,
-                                 bool aNeedsFocus = false) override;
+  void SetFocusedElement(mozilla::dom::Element* aElement,
+                         uint32_t aFocusMethod = 0, bool aNeedsFocus = false,
+                         bool aWillShowOutline = false) override;
 
-  virtual uint32_t GetFocusMethod() override;
+  uint32_t GetFocusMethod() override;
 
-  virtual bool ShouldShowFocusRing() override;
+  bool ShouldShowFocusRing() override;
 
-  virtual void SetKeyboardIndicators(
-      UIStateChangeType aShowFocusRings) override;
+  void SetKeyboardIndicators(UIStateChangeType aShowFocusRings) override;
 
  public:
-  virtual already_AddRefed<nsPIWindowRoot> GetTopWindowRoot() override;
+  already_AddRefed<nsPIWindowRoot> GetTopWindowRoot() override;
 
  protected:
   void NotifyWindowIDDestroyed(const char* aTopic);
 
   void ClearStatus();
 
-  virtual void UpdateParentTarget() override;
+  void UpdateParentTarget() override;
 
   void InitializeShowFocusRings();
 

dom/base/nsPIDOMWindow.h

@@ -481,10 +481,22 @@ class nsPIDOMWindowInner : public mozIDOMWindow {
    * DO NOT CALL EITHER OF THESE METHODS DIRECTLY. USE THE FOCUS MANAGER
    * INSTEAD.
    */
-  inline mozilla::dom::Element* GetFocusedElement() const;
+  mozilla::dom::Element* GetFocusedElement() const {
+    return mFocusedElement.get();
+  }
+
   virtual void SetFocusedElement(mozilla::dom::Element* aElement,
                                  uint32_t aFocusMethod = 0,
-                                 bool aNeedsFocus = false) = 0;
+                                 bool aNeedsFocus = false,
+                                 bool aWillShowOutline = false) = 0;
+  /**
+   * Get whether the focused element did show outlines when it was focused.
+   *
+   * Only for the focus manager. Returns false if there was no focused element.
+   */
+  bool FocusedElementShowedOutline() const {
+    return mFocusedElementShowedOutlines;
+  }
 
   /**
    * Retrieves the method that was used to focus the current node.
@@ -669,6 +681,8 @@ class nsPIDOMWindowInner : public mozIDOMWindow {
   // notification.
   bool mHasNotifiedGlobalCreated;
 
+  bool mFocusedElementShowedOutlines = false;
+
   uint32_t mMarkedCCGeneration;
 
   // mTopInnerWindow is used for tab-wise check by timeout throttling. It could
@@ -938,9 +952,17 @@ class nsPIDOMWindowOuter : public mozIDOMWindowProxy {
    * INSTEAD.
    */
   inline mozilla::dom::Element* GetFocusedElement() const;
+
   virtual void SetFocusedElement(mozilla::dom::Element* aElement,
                                  uint32_t aFocusMethod = 0,
-                                 bool aNeedsFocus = false) = 0;
+                                 bool aNeedsFocus = false,
+                                 bool aWillShowOutline = false) = 0;
+  /**
+   * Get whether the focused element did show outlines when it was focused.
+   *
+   * Only for the focus manager. Returns false if there was no focused element.
+   */
+  bool FocusedElementShowedOutline() const;
 
   /**
    * Retrieves the method that was used to focus the current node.

dom/base/nsPIDOMWindowInlines.h

@@ -81,8 +81,8 @@ inline mozilla::dom::Element* nsPIDOMWindowOuter::GetFocusedElement() const {
   return mInnerWindow ? mInnerWindow->GetFocusedElement() : nullptr;
 }
 
-inline mozilla::dom::Element* nsPIDOMWindowInner::GetFocusedElement() const {
-  return mFocusedElement;
+inline bool nsPIDOMWindowOuter::FocusedElementShowedOutline() const {
+  return mInnerWindow && mInnerWindow->FocusedElementShowedOutline();
 }
 
 #endif

dom/base/test/browser.ini

@@ -76,3 +76,4 @@ support-files =
   file_bug1554070_1.html
   file_bug1554070_2.html
 [browser_chromeutils_getalldomprocesses.js]
+[browser_outline_refocus.js]

dom/base/test/browser_outline_refocus.js

@@ -0,0 +1,61 @@
+const URL = `data:text/html,<a target="_blank" href="http://example.com">Click me</a>`;
+
+async function test_browser_outline_refocus(
+  aMessage,
+  aShouldFocusBeVisible,
+  aOpenTabCallback
+) {
+  await BrowserTestUtils.withNewTab(URL, async function(browser) {
+    let tab = gBrowser.getTabForBrowser(browser);
+    let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser);
+
+    await aOpenTabCallback(browser);
+
+    info("waiting for new tab");
+    let newTab = await newTabPromise;
+
+    is(gBrowser.selectedTab, newTab, "Should've switched to the new tab");
+
+    info("switching back");
+    await BrowserTestUtils.switchTab(gBrowser, tab);
+
+    info("checking focus");
+    let [wasFocused, wasFocusVisible] = await SpecialPowers.spawn(
+      browser,
+      [],
+      () => {
+        let link = content.document.querySelector("a");
+        return [link.matches(":focus"), link.matches(":focus-visible")];
+      }
+    );
+
+    ok(wasFocused, "Link should be refocused");
+    is(wasFocusVisible, aShouldFocusBeVisible, aMessage);
+
+    info("closing tab");
+    await BrowserTestUtils.removeTab(newTab);
+  });
+}
+
+add_task(async function browser_outline_refocus_mouse() {
+  await test_browser_outline_refocus(
+    "Link shouldn't show outlines since it was originally focused by mouse",
+    false,
+    function(aBrowser) {
+      info("clicking on link");
+      return BrowserTestUtils.synthesizeMouseAtCenter("a", {}, aBrowser);
+    }
+  );
+});
+
+add_task(async function browser_outline_refocus_key() {
+  await test_browser_outline_refocus(
+    "Link should show outlines since it was originally focused by keyboard",
+    true,
+    function(aBrowser) {
+      info("Navigating via keyboard");
+      EventUtils.sendKey("tab");
+      EventUtils.sendKey("return");
+    }
+  );
+});