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:

  https://github.com/w3c/csswg-drafts/issues/5885
  https://github.com/web-platform-tests/wpt/pull/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

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) {

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();

dom/base/nsGlobalWindowInner.cpp

@@ -4341,8 +4341,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;
@@ -4352,7 +4351,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);
@@ -4361,13 +4359,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;
     }
   }
 

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;
 

dom/base/nsGlobalWindowOuter.cpp

@@ -6711,10 +6711,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() {

dom/base/nsGlobalWindowOuter.h

@@ -891,8 +891,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;
 

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;
 
@@ -953,14 +948,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.

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

dom/tests/mochitest/general/test_focusrings.xhtml

@@ -46,7 +46,7 @@ function runTest()
   }
 
   // make sure that a focus ring appears on the focused button
-  if (navigator.platform.includes("Mac")) {
+  if (isMac) {
     var focusedButton = $("b3");
     ok(compareSnapshots(snapShot(focusedButton), snapShot($("b2")), true)[0], "unfocused shows no ring");
     focusedButton.focus();
@@ -103,10 +103,10 @@ function testMacFocusesFormControl()
 
 var htmlElements = [
   "<button id='elem'>Button</button>",
-  "<input id='elem' class='canfocus'>",
-  "<input id='elem' type='password' class='canfocus'>",
   "<input id='elem' type='button'>",
   "<input id='elem' type='checkbox'>",
+  "<input id='elem' class='canfocus'>",
+  "<input id='elem' type='password' class='canfocus'>",
   "<textarea id='elem' class='canfocus'></textarea>",
   "<select id='elem' class='canfocus'><option>One</select>",
   "<select id='elem' rows='5' class='canfocus'><option>One</select>",
@@ -167,7 +167,10 @@ function testHTMLElements(list, isMac, expectedNoRingsOnWin)
     if (childdoc.activeElement)
       childdoc.activeElement.blur();
 
-    ringSize = (elem.localName == "a" ? "0" : (expectedNoRingsOnWin ? 2 : 1)) + "px";
+    ringSize = mouseRingSize;
+    if (isMac && !shouldFocus && elem.localName != "a") {
+      ringSize = "1px";
+    }
 
     elem.focus();
     is(childdoc.activeElement, elem, "focus() on " + list[e]);

layout/reftests/bugs/reftest.list

@@ -1909,7 +1909,7 @@ fuzzy-if(webrender,0-128,0-22) == 1155828-1.html 1155828-1-ref.html # bug 164652
 fuzzy-if(skiaContent,0-7,0-84) == 1156129-1.html 1156129-1-ref.html
 pref(dom.use_xbl_scopes_for_remote_xul,true) HTTP(..) == 1157127-1.html 1157127-1-ref.html
 fuzzy-if(Android,0-6,0-6) == 1169331-1.html 1169331-1-ref.html
-fuzzy(0-1,0-74) random-if(Android||gtkWidget) fuzzy-if(winWidget&&!nativeThemePref,0-68,0-28) == 1174332-1.html 1174332-1-ref.html # bug 1312658
+fuzzy(0-3,0-95) random-if(Android||gtkWidget) fuzzy-if(winWidget&&!nativeThemePref,0-68,0-257) == 1174332-1.html 1174332-1-ref.html # bug 1312658, plus on webrender fallback vs. non-fallback codepath causes fuzz
 == 1179078-1.html 1179078-1-ref.html
 == 1179288-1.html 1179288-1-ref.html
 == 1190635-1.html 1190635-1-ref.html