Bug 1699154 - Tweak focusring heuristics for script focus. r=edgar

What we implemented before this patch was basically what the heuristics in the spec said, which used to be normative: https://drafts.csswg.org/selectors/#the-focus-visible-pseudo That has become non-normative and there's ongoing discussion on what should happen for cases like this in: w3c/csswg-drafts#5885 web-platform-tests/wpt#27806 There seems to be agreement on that WPT issue on cases like this one, so let's make it work. Differential Revision: https://phabricator.services.mozilla.com/D108805
mozilla · Mar 18, 2021 · 1cad7af · 1cad7af
1 parent e872192
commit 1cad7af
Show file tree

Hide file tree

Showing 9 changed files with 33 additions and 61 deletions.
diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp
@@ -1225,16 +1225,14 @@ nsresult nsFocusManager::FocusPlugin(Element* aPlugin) {
 }
 
 nsFocusManager::BlurredElementInfo::BlurredElementInfo(Element& aElement)
-    : mElement(aElement),
-      mHadRing(aElement.State().HasState(NS_EVENT_STATE_FOCUSRING)) {}
+    : mElement(aElement) {}
 
 nsFocusManager::BlurredElementInfo::~BlurredElementInfo() = default;
 
 // https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo
-static bool ShouldMatchFocusVisible(
-    nsPIDOMWindowOuter* aWindow, const Element& aElement, int32_t aFocusFlags,
-    const Maybe<nsFocusManager::BlurredElementInfo>& aBlurredElementInfo,
-    bool aIsRefocus, bool aRefocusedElementUsedToShowOutline) {
+static bool ShouldMatchFocusVisible(nsPIDOMWindowOuter* aWindow,
+                                    const Element& aElement,
+                                    int32_t aFocusFlags) {
   // If we were explicitly requested to show the ring, do it.
   if (aFocusFlags & nsIFocusManager::FLAG_SHOWRING) {
     return true;
@@ -1269,20 +1267,9 @@ static bool ShouldMatchFocusVisible(
       // :focus).
       return true;
     case InputContextAction::CAUSE_UNKNOWN:
-      // If the active element matches :focus-visible, and a script causes focus
-      // to move elsewhere, the newly focused element should match
-      // :focus-visible.
-      //
-      // 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;
+      // We render outlines if the last "known" focus method was by key or there
+      // was no previous known focus method, otherwise we don't.
+      return aWindow->UnknownFocusMethodShouldShowOutline();
     case InputContextAction::CAUSE_MOUSE:
     case InputContextAction::CAUSE_TOUCH:
     case InputContextAction::CAUSE_LONGPRESS:
@@ -2543,13 +2530,9 @@ void nsFocusManager::Focus(
                                 !IsNonFocusableRoot(aElement);
     const bool isRefocus = focusedNode && focusedNode == aElement;
     const bool shouldShowFocusRing =
-        sendFocusEvent &&
-        ShouldMatchFocusVisible(
-            aWindow, *aElement, aFlags, aBlurredElementInfo, isRefocus,
-            isRefocus && aWindow->FocusedElementShowedOutline());
+        sendFocusEvent && ShouldMatchFocusVisible(aWindow, *aElement, aFlags);
 
-    aWindow->SetFocusedElement(aElement, focusMethod, false,
-                               shouldShowFocusRing);
+    aWindow->SetFocusedElement(aElement, focusMethod, false);
 
     // if the focused element changed, scroll it into view
     if (aElement && aFocusChanged) {

diff --git a/dom/base/nsFocusManager.h b/dom/base/nsFocusManager.h
@@ -301,7 +301,6 @@ class nsFocusManager final : public nsIFocusManager,
 
   struct BlurredElementInfo {
     const mozilla::OwningNonNull<mozilla::dom::Element> mElement;
-    const bool mHadRing;
 
     explicit BlurredElementInfo(mozilla::dom::Element&);
     ~BlurredElementInfo();

diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp
@@ -4336,8 +4336,7 @@ void nsGlobalWindowInner::StopVRActivity() {
 
 void nsGlobalWindowInner::SetFocusedElement(Element* aElement,
                                             uint32_t aFocusMethod,
-                                            bool aNeedsFocus,
-                                            bool aWillShowOutline) {
+                                            bool aNeedsFocus) {
   if (aElement && aElement->GetComposedDoc() != mDoc) {
     NS_WARNING("Trying to set focus to a node from a wrong document");
     return;
@@ -4347,7 +4346,6 @@ 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);
@@ -4356,13 +4354,15 @@ void nsGlobalWindowInner::SetFocusedElement(Element* aElement,
     mFocusMethod = aFocusMethod & FOCUSMETHOD_MASK;
   }
 
-  mFocusedElementShowedOutlines = aWillShowOutline;
-
   if (mFocusedElement) {
     // if a node was focused by a keypress, turn on focus rings for the
     // window.
     if (mFocusMethod & nsIFocusManager::FLAG_BYKEY) {
+      mUnknownFocusMethodShouldShowOutline = true;
       mFocusByKeyOccurred = true;
+    } else if (nsFocusManager::GetFocusMoveActionCause(mFocusMethod) !=
+               widget::InputContextAction::CAUSE_UNKNOWN) {
+      mUnknownFocusMethodShouldShowOutline = false;
     }
   }
 

diff --git a/dom/base/nsGlobalWindowInner.h b/dom/base/nsGlobalWindowInner.h
@@ -1130,8 +1130,8 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
   bool IsInModalState();
 
   void SetFocusedElement(mozilla::dom::Element* aElement,
-                         uint32_t aFocusMethod = 0, bool aNeedsFocus = false,
-                         bool aWillShowOutline = false) override;
+                         uint32_t aFocusMethod = 0,
+                         bool aNeedsFocus = false) override;
 
   uint32_t GetFocusMethod() override;
 

diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp
@@ -6710,10 +6710,9 @@ void nsGlobalWindowOuter::SetChromeEventHandler(
 
 void nsGlobalWindowOuter::SetFocusedElement(Element* aElement,
                                             uint32_t aFocusMethod,
-                                            bool aNeedsFocus,
-                                            bool aWillShowOutline) {
-  FORWARD_TO_INNER_VOID(SetFocusedElement, (aElement, aFocusMethod, aNeedsFocus,
-                                            aWillShowOutline));
+                                            bool aNeedsFocus) {
+  FORWARD_TO_INNER_VOID(SetFocusedElement,
+                        (aElement, aFocusMethod, aNeedsFocus));
 }
 
 uint32_t nsGlobalWindowOuter::GetFocusMethod() {

diff --git a/dom/base/nsGlobalWindowOuter.h b/dom/base/nsGlobalWindowOuter.h
@@ -895,8 +895,8 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget,
                                            nsIBaseWindow* aWindow);
 
   void SetFocusedElement(mozilla::dom::Element* aElement,
-                         uint32_t aFocusMethod = 0, bool aNeedsFocus = false,
-                         bool aWillShowOutline = false) override;
+                         uint32_t aFocusMethod = 0,
+                         bool aNeedsFocus = false) override;
 
   uint32_t GetFocusMethod() override;
 

diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h
@@ -487,15 +487,10 @@ class nsPIDOMWindowInner : public mozIDOMWindow {
 
   virtual void SetFocusedElement(mozilla::dom::Element* aElement,
                                  uint32_t aFocusMethod = 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;
+                                 bool aNeedsFocus = false) = 0;
+
+  bool UnknownFocusMethodShouldShowOutline() const {
+    return mUnknownFocusMethodShouldShowOutline;
   }
 
   /**
@@ -681,7 +676,7 @@ class nsPIDOMWindowInner : public mozIDOMWindow {
   // notification.
   bool mHasNotifiedGlobalCreated;
 
-  bool mFocusedElementShowedOutlines = false;
+  bool mUnknownFocusMethodShouldShowOutline = true;
 
   uint32_t mMarkedCCGeneration;
 
@@ -955,14 +950,12 @@ class nsPIDOMWindowOuter : public mozIDOMWindowProxy {
 
   virtual void SetFocusedElement(mozilla::dom::Element* aElement,
                                  uint32_t aFocusMethod = 0,
-                                 bool aNeedsFocus = false,
-                                 bool aWillShowOutline = false) = 0;
+                                 bool aNeedsFocus = 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.
+   * Get whether a focused element focused by unknown reasons (like script
+   * focus) should match the :focus-visible pseudo-class.
    */
-  bool FocusedElementShowedOutline() const;
+  bool UnknownFocusMethodShouldShowOutline() const;
 
   /**
    * Retrieves the method that was used to focus the current node.

diff --git a/dom/base/nsPIDOMWindowInlines.h b/dom/base/nsPIDOMWindowInlines.h
@@ -81,8 +81,8 @@ inline mozilla::dom::Element* nsPIDOMWindowOuter::GetFocusedElement() const {
   return mInnerWindow ? mInnerWindow->GetFocusedElement() : nullptr;
 }
 
-inline bool nsPIDOMWindowOuter::FocusedElementShowedOutline() const {
-  return mInnerWindow && mInnerWindow->FocusedElementShowedOutline();
+inline bool nsPIDOMWindowOuter::UnknownFocusMethodShouldShowOutline() const {
+  return mInnerWindow && mInnerWindow->UnknownFocusMethodShouldShowOutline();
 }
 
 #endif
diff --git a/dom/tests/mochitest/general/test_focusrings.xhtml b/dom/tests/mochitest/general/test_focusrings.xhtml
@@ -167,11 +167,9 @@ function testHTMLElements(list, isMac, expectedNoRingsOnWin)
     if (childdoc.activeElement)
       childdoc.activeElement.blur();
 
-    ringSize = (elem.localName == "a" ? "0" : (expectedNoRingsOnWin ? 2 : 1)) + "px";
-
     elem.focus();
     is(childdoc.activeElement, elem, "focus() on " + list[e]);
-    is(childwin.getComputedStyle(elem).outlineWidth, ringSize,
+    is(childwin.getComputedStyle(elem).outlineWidth, mouseRingSize,
        "focus() on " + list[e] + " ring");
 
     childdoc.activeElement.blur();