Bug 1638821 - Make the decision to whether paint focus rings depend on the theme, not the platform. r=spohl

The current code assumes that nsNativeTheme is in use which breaks with
the non-native theme.

Instead of hackily remove the FOCUS bit, make the theme check for the
FOCUSRING bit, which is the right thing to check for anyway.

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

widget/cocoa/nsNativeThemeCocoa.mm

@@ -1220,13 +1220,30 @@ void nsNativeThemeCocoa::DrawMenuSeparator(CGContextRef cgContext, const CGRect&
   HIThemeDrawMenuSeparator(&inBoxRect, &inBoxRect, &midi, cgContext, HITHEME_ORIENTATION);
 }
 
+static bool ShouldUnconditionallyDrawFocusRingIfFocused(nsIFrame* aFrame) {
+  // Mac always draws focus rings for textboxes and lists.
+  switch (aFrame->StyleDisplay()->mAppearance) {
+    case StyleAppearance::MenulistTextfield:
+    case StyleAppearance::NumberInput:
+    case StyleAppearance::Textfield:
+    case StyleAppearance::Textarea:
+    case StyleAppearance::Searchfield:
+    case StyleAppearance::Listbox:
+      return true;
+    default:
+      return false;
+  }
+}
+
 nsNativeThemeCocoa::ControlParams nsNativeThemeCocoa::ComputeControlParams(
     nsIFrame* aFrame, EventStates aEventState) {
   ControlParams params;
   params.disabled = IsDisabled(aFrame, aEventState);
   params.insideActiveWindow = FrameIsInActiveWindow(aFrame);
   params.pressed = aEventState.HasAllStates(NS_EVENT_STATE_ACTIVE | NS_EVENT_STATE_HOVER);
-  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUS);
+  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUS) &&
+                   (aEventState.HasState(NS_EVENT_STATE_FOCUSRING) ||
+                    ShouldUnconditionallyDrawFocusRingIfFocused(aFrame));
   params.rtl = IsFrameRTL(aFrame);
   return params;
 }
@@ -2035,7 +2052,7 @@ nsNativeThemeCocoa::ScaleParams nsNativeThemeCocoa::ComputeXULScaleParams(nsIFra
       aFrame->GetContent()->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::dir,
                                                      NS_LITERAL_STRING("reverse"), eCaseMatters);
   params.insideActiveWindow = FrameIsInActiveWindow(aFrame);
-  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUS);
+  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUSRING);
   params.disabled = IsDisabled(aFrame, aEventState);
   params.horizontal = aIsHorizontal;
   return params;
@@ -2058,7 +2075,7 @@ Maybe<nsNativeThemeCocoa::ScaleParams> nsNativeThemeCocoa::ComputeHTMLScaleParam
   params.max = 1000;
   params.reverse = !isHorizontal || rangeFrame->IsRightToLeft();
   params.insideActiveWindow = FrameIsInActiveWindow(aFrame);
-  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUS);
+  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUSRING);
   params.disabled = IsDisabled(aFrame, aEventState);
   params.horizontal = isHorizontal;
   return Some(params);
@@ -2157,7 +2174,7 @@ nsNativeThemeCocoa::SegmentParams nsNativeThemeCocoa::ComputeSegmentParams(
   params.insideActiveWindow = FrameIsInActiveWindow(aFrame);
   params.pressed = IsPressedButton(aFrame);
   params.selected = IsSelectedButton(aFrame);
-  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUS);
+  params.focused = aEventState.HasState(NS_EVENT_STATE_FOCUSRING);
   bool isRTL = IsFrameRTL(aFrame);
   nsIFrame* left = GetAdjacentSiblingFrameWithSameAppearance(aFrame, isRTL);
   nsIFrame* right = GetAdjacentSiblingFrameWithSameAppearance(aFrame, !isRTL);
@@ -2920,6 +2937,7 @@ Maybe<nsNativeThemeCocoa::WidgetInfo> nsNativeThemeCocoa::ComputeWidgetInfo(
     case StyleAppearance::MenulistTextfield:
     case StyleAppearance::Textfield:
     case StyleAppearance::NumberInput: {
+      // See ShouldUnconditionallyDrawFocusRingIfFocused.
       bool isFocused = eventState.HasState(NS_EVENT_STATE_FOCUS);
       // XUL textboxes set the native appearance on the containing box, while
       // concrete focus is set on the html:input element within it. We can

widget/nsNativeTheme.cpp

@@ -62,40 +62,16 @@ EventStates nsNativeTheme::GetContentState(nsIFrame* aFrame,
     }
   }
 
-  if (isXULCheckboxRadio && aAppearance == StyleAppearance::Radio) {
+  if (isXULCheckboxRadio && aAppearance == StyleAppearance::Radio &&
-    if (IsFocused(aFrame)) {
+      IsFocused(aFrame)) {
-      flags |= NS_EVENT_STATE_FOCUS;
+    flags |= NS_EVENT_STATE_FOCUS;
-      nsPIDOMWindowOuter* window =
+    nsPIDOMWindowOuter* window =
-          aFrame->GetContent()->OwnerDoc()->GetWindow();
+        aFrame->GetContent()->OwnerDoc()->GetWindow();
-      if (window && window->ShouldShowFocusRing()) {
+    if (window && window->ShouldShowFocusRing()) {
-        flags |= NS_EVENT_STATE_FOCUSRING;
+      flags |= NS_EVENT_STATE_FOCUSRING;
-      }
     }
   }
 
-  // On Windows and Mac, only draw focus rings if they should be shown. This
-  // means that focus rings are only shown once the keyboard has been used to
-  // focus something in the window.
-#if defined(XP_MACOSX)
-  // Mac always draws focus rings for textboxes and lists.
-  if (aAppearance == StyleAppearance::MenulistTextfield ||
-      aAppearance == StyleAppearance::NumberInput ||
-      aAppearance == StyleAppearance::Textfield ||
-      aAppearance == StyleAppearance::Textarea ||
-      aAppearance == StyleAppearance::Searchfield ||
-      aAppearance == StyleAppearance::Listbox) {
-    return flags;
-  }
-#endif
-#if defined(XP_WIN)
-  // On Windows, focused buttons are always drawn as such by the native theme.
-  if (aAppearance == StyleAppearance::Button) return flags;
-#endif
-#if defined(XP_MACOSX) || defined(XP_WIN)
-  if (!flags.HasState(NS_EVENT_STATE_FOCUSRING)) {
-    flags &= ~NS_EVENT_STATE_FOCUS;
-  }
-#endif
   return flags;
 }
 

widget/windows/nsNativeThemeWin.cpp

@@ -803,11 +803,24 @@ int32_t nsNativeThemeWin::StandardGetState(nsIFrame* aFrame,
                                            StyleAppearance aAppearance,
                                            bool wantFocused) {
   EventStates eventState = GetContentState(aFrame, aAppearance);
-  if (eventState.HasAllStates(NS_EVENT_STATE_HOVER | NS_EVENT_STATE_ACTIVE))
+  if (eventState.HasAllStates(NS_EVENT_STATE_HOVER | NS_EVENT_STATE_ACTIVE)) {
     return TS_ACTIVE;
-  if (eventState.HasState(NS_EVENT_STATE_HOVER)) return TS_HOVER;
+  }
-  if (wantFocused && eventState.HasState(NS_EVENT_STATE_FOCUS))
+  if (eventState.HasState(NS_EVENT_STATE_HOVER)) {
-    return TS_FOCUSED;
+    return TS_HOVER;
+  }
+  if (wantFocused) {
+    if (eventState.HasState(NS_EVENT_STATE_FOCUSRING)) {
+      return TS_FOCUSED;
+    }
+    // On Windows, focused buttons are always drawn as such by the native
+    // theme, that's why we check NS_EVENT_STATE_FOCUS instead of
+    // NS_EVENT_STATE_FOCUSRING.
+    if (aAppearance == StyleAppearance::Button &&
+        eventState.HasState(NS_EVENT_STATE_FOCUS)) {
+      return TS_FOCUSED;
+    }
+  }
 
   return TS_NORMAL;
 }
@@ -924,7 +937,7 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
         if (content && content->IsXULElement() && IsFocused(aFrame))
           aState = TFS_EDITBORDER_FOCUSED;
         else if (eventState.HasAtLeastOneOfStates(NS_EVENT_STATE_ACTIVE |
-                                                  NS_EVENT_STATE_FOCUS))
+                                                  NS_EVENT_STATE_FOCUSRING))
           aState = TFS_EDITBORDER_FOCUSED;
         else if (eventState.HasState(NS_EVENT_STATE_HOVER))
           aState = TFS_EDITBORDER_HOVER;
@@ -1106,7 +1119,7 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
                                          // canceled when you move outside the
                                          // thumb.
           aState = TS_ACTIVE;
-        else if (eventState.HasState(NS_EVENT_STATE_FOCUS))
+        else if (eventState.HasState(NS_EVENT_STATE_FOCUSRING))
           aState = TKP_FOCUSED;
         else if (eventState.HasState(NS_EVENT_STATE_HOVER))
           aState = TS_HOVER;
@@ -1252,8 +1265,8 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
       } else if (IsOpenButton(aFrame)) {
         aState = TS_ACTIVE;
       } else {
-        if (useDropBorder &&
+        if (useDropBorder && (eventState.HasState(NS_EVENT_STATE_FOCUSRING) ||
-            (eventState.HasState(NS_EVENT_STATE_FOCUS) || IsFocused(aFrame)))
+                              IsFocused(aFrame)))
           aState = TS_ACTIVE;
         else if (eventState.HasAllStates(NS_EVENT_STATE_HOVER |
                                          NS_EVENT_STATE_ACTIVE))
@@ -1882,7 +1895,7 @@ RENDER_AGAIN:
       aAppearance == StyleAppearance::ScaleVertical) {
     EventStates contentState = GetContentState(aFrame, aAppearance);
 
-    if (contentState.HasState(NS_EVENT_STATE_FOCUS)) {
+    if (contentState.HasState(NS_EVENT_STATE_FOCUSRING)) {
       POINT vpOrg;
       HPEN hPen = nullptr;
 
@@ -3119,6 +3132,9 @@ nsresult nsNativeThemeWin::ClassicGetThemePartAndState(
             aFocused = true;
           }
         }
+        // On Windows, focused buttons are always drawn as such by the native
+        // theme, that's why we check NS_EVENT_STATE_FOCUS instead of
+        // NS_EVENT_STATE_FOCUSRING.
         if (contentState.HasState(NS_EVENT_STATE_FOCUS) ||
             (aState == DFCS_BUTTONPUSH && IsDefaultButton(aFrame))) {
           aFocused = true;
@@ -3155,7 +3171,7 @@ nsresult nsNativeThemeWin::ClassicGetThemePartAndState(
 
       contentState = GetContentState(aFrame, aAppearance);
       if (!content->IsXULElement() &&
-          contentState.HasState(NS_EVENT_STATE_FOCUS)) {
+          contentState.HasState(NS_EVENT_STATE_FOCUSRING)) {
         aFocused = true;
       }