Bug 1734723 - Don't use transparent borders for checkbox/radio when focused. r=mstange
authorEmilio Cobos Alvarez <emilio@crisal.io>
Fri, 08 Oct 2021 10:59:02 +0000
changeset 595199 7e2386b59ca2a3ced502a49447f4df26d3943a85
parent 595198 eea2d5968d6cadf9c629b1cde83af3b2ff4d39c2
child 595200 dc2ca800da7c1cdfd2e1d7705f7bcf53b1bc6e5c
push id38862
push userctuns@mozilla.com
push dateFri, 08 Oct 2021 21:42:24 +0000
treeherdermozilla-central@798c43651cb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1734723
milestone95.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1734723 - Don't use transparent borders for checkbox/radio when focused. r=mstange As those don't draw the outline on top. Differential Revision: https://phabricator.services.mozilla.com/D127890
widget/nsNativeBasicTheme.cpp
widget/nsNativeBasicTheme.h
--- a/widget/nsNativeBasicTheme.cpp
+++ b/widget/nsNativeBasicTheme.cpp
@@ -408,51 +408,52 @@ std::pair<sRGBColor, sRGBColor> nsNative
         aState.HasAllStates(NS_EVENT_STATE_HOVER | NS_EVENT_STATE_ACTIVE);
     bool isHovered = aState.HasState(NS_EVENT_STATE_HOVER);
     const auto& color = isActive    ? aColors.Accent().GetDarker()
                         : isHovered ? aColors.Accent().GetDark()
                                     : aColors.Accent().Get();
     return std::make_pair(color, color);
   }
 
-  return ComputeTextfieldColors(aState, aColors);
+  return ComputeTextfieldColors(aState, aColors, OutlineCoversBorder::No);
 }
 
 sRGBColor nsNativeBasicTheme::ComputeCheckmarkColor(const EventStates& aState,
                                                     const Colors& aColors) {
   if (aColors.HighContrast()) {
     return aColors.System(StyleSystemColor::Selecteditemtext);
   }
   if (aState.HasState(NS_EVENT_STATE_DISABLED)) {
     return sColorWhiteAlpha80;
   }
   return aColors.Accent().GetForeground();
 }
 
-sRGBColor nsNativeBasicTheme::ComputeBorderColor(const EventStates& aState,
-                                                 const Colors& aColors) {
+sRGBColor nsNativeBasicTheme::ComputeBorderColor(
+    const EventStates& aState, const Colors& aColors,
+    OutlineCoversBorder aOutlineCoversBorder) {
   bool isDisabled = aState.HasState(NS_EVENT_STATE_DISABLED);
   if (aColors.HighContrast()) {
     return aColors.System(isDisabled ? StyleSystemColor::Graytext
                                      : StyleSystemColor::Buttontext);
   }
   bool isActive =
       aState.HasAllStates(NS_EVENT_STATE_HOVER | NS_EVENT_STATE_ACTIVE);
   bool isHovered = aState.HasState(NS_EVENT_STATE_HOVER);
   bool isFocused = aState.HasState(NS_EVENT_STATE_FOCUSRING);
   if (isDisabled) {
     return sColorGrey40Alpha50;
   }
-  if (isFocused) {
-    // We draw the outline over the border for all controls that call into this,
-    // so to prevent issues where the border shows underneath if it snaps in the
-    // wrong direction, we use a transparent border. An alternative to this is
-    // ensuring that we snap the offset in PaintRoundedFocusRect the same was a
-    // we snap border widths, so that negative offsets are guaranteed to cover
-    // the border. But this looks harder to mess up.
+  if (isFocused && aOutlineCoversBorder == OutlineCoversBorder::Yes) {
+    // If we draw the outline over the border, prevent issues where the border
+    // shows underneath if it snaps in the wrong direction by using a
+    // transparent border. An alternative to this is ensuring that we snap the
+    // offset in PaintRoundedFocusRect the same was a we snap border widths, so
+    // that negative offsets are guaranteed to cover the border.
+    // But this looks harder to mess up.
     return sTransparent;
   }
   bool dark = aColors.IsDark();
   if (isActive) {
     return dark ? sColorGrey20 : sColorGrey60;
   }
   if (isHovered) {
     return dark ? sColorGrey30 : sColorGrey50;
@@ -475,39 +476,42 @@ std::pair<sRGBColor, sRGBColor> nsNative
       return aColors.System(StyleSystemColor::MozButtonactiveface);
     }
     if (isHovered) {
       return aColors.System(StyleSystemColor::MozButtonhoverface);
     }
     return aColors.System(StyleSystemColor::Buttonface);
   }();
 
-  const sRGBColor borderColor = ComputeBorderColor(aState, aColors);
+  const sRGBColor borderColor =
+      ComputeBorderColor(aState, aColors, OutlineCoversBorder::Yes);
   return std::make_pair(backgroundColor, borderColor);
 }
 
 // NOTE: This should be kept in sync with forms.css, see the comment in the
 // input:autofill rule.
 constexpr nscolor kAutofillColor = NS_RGBA(255, 249, 145, 128);
 
 std::pair<sRGBColor, sRGBColor> nsNativeBasicTheme::ComputeTextfieldColors(
-    const EventStates& aState, const Colors& aColors) {
+    const EventStates& aState, const Colors& aColors,
+    OutlineCoversBorder aOutlineCoversBorder) {
   nscolor backgroundColor = [&] {
     if (aState.HasState(NS_EVENT_STATE_DISABLED)) {
       return aColors.SystemNs(StyleSystemColor::MozDisabledfield);
     }
     return aColors.SystemNs(StyleSystemColor::Field);
   }();
 
   if (aState.HasState(NS_EVENT_STATE_AUTOFILL) &&
       StaticPrefs::layout_css_autofill_background()) {
     backgroundColor = NS_ComposeColors(backgroundColor, kAutofillColor);
   }
 
-  const sRGBColor borderColor = ComputeBorderColor(aState, aColors);
+  const sRGBColor borderColor =
+      ComputeBorderColor(aState, aColors, aOutlineCoversBorder);
   return std::make_pair(sRGBColor::FromABGR(backgroundColor), borderColor);
 }
 
 std::pair<sRGBColor, sRGBColor> nsNativeBasicTheme::ComputeRangeProgressColors(
     const EventStates& aState, const Colors& aColors) {
   if (aColors.HighContrast()) {
     return aColors.SystemPair(StyleSystemColor::Selecteditem,
                               StyleSystemColor::Buttontext);
@@ -1246,17 +1250,18 @@ void nsNativeBasicTheme::PaintRadioContr
 }
 
 template <typename PaintBackendData>
 void nsNativeBasicTheme::PaintTextField(PaintBackendData& aPaintData,
                                         const LayoutDeviceRect& aRect,
                                         const EventStates& aState,
                                         const Colors& aColors,
                                         DPIRatio aDpiRatio) {
-  auto [backgroundColor, borderColor] = ComputeTextfieldColors(aState, aColors);
+  auto [backgroundColor, borderColor] =
+      ComputeTextfieldColors(aState, aColors, OutlineCoversBorder::Yes);
 
   const CSSCoord radius = 2.0f;
 
   PaintRoundedRectWithRadius(aPaintData, aRect, backgroundColor, borderColor,
                              kTextFieldBorderWidth, radius, aDpiRatio);
 
   if (aState.HasState(NS_EVENT_STATE_FOCUSRING)) {
     PaintRoundedFocusRect(aPaintData, aRect, aColors, aDpiRatio,
@@ -1267,17 +1272,18 @@ void nsNativeBasicTheme::PaintTextField(
 
 template <typename PaintBackendData>
 void nsNativeBasicTheme::PaintListbox(PaintBackendData& aPaintData,
                                       const LayoutDeviceRect& aRect,
                                       const EventStates& aState,
                                       const Colors& aColors,
                                       DPIRatio aDpiRatio) {
   const CSSCoord radius = 2.0f;
-  auto [backgroundColor, borderColor] = ComputeTextfieldColors(aState, aColors);
+  auto [backgroundColor, borderColor] =
+      ComputeTextfieldColors(aState, aColors, OutlineCoversBorder::Yes);
 
   PaintRoundedRectWithRadius(aPaintData, aRect, backgroundColor, borderColor,
                              kMenulistBorderWidth, radius, aDpiRatio);
 
   if (aState.HasState(NS_EVENT_STATE_FOCUSRING)) {
     PaintRoundedFocusRect(aPaintData, aRect, aColors, aDpiRatio,
                           radius + kMenulistBorderWidth, -kMenulistBorderWidth);
   }
--- a/widget/nsNativeBasicTheme.h
+++ b/widget/nsNativeBasicTheme.h
@@ -184,23 +184,26 @@ class nsNativeBasicTheme : protected nsN
 
   // Whether we should use system colors (for high contrast mode).
   static bool ShouldBeHighContrast(const nsPresContext&);
 
   std::pair<sRGBColor, sRGBColor> ComputeCheckboxColors(const EventStates&,
                                                         StyleAppearance,
                                                         const Colors&);
   sRGBColor ComputeCheckmarkColor(const EventStates&, const Colors&);
-  sRGBColor ComputeBorderColor(const EventStates&, const Colors&);
+  enum class OutlineCoversBorder : bool { No, Yes };
+  sRGBColor ComputeBorderColor(const EventStates&, const Colors&,
+                               OutlineCoversBorder);
 
   std::pair<sRGBColor, sRGBColor> ComputeButtonColors(const EventStates&,
                                                       const Colors&,
                                                       nsIFrame* = nullptr);
   std::pair<sRGBColor, sRGBColor> ComputeTextfieldColors(const EventStates&,
-                                                         const Colors&);
+                                                         const Colors&,
+                                                         OutlineCoversBorder);
   std::pair<sRGBColor, sRGBColor> ComputeRangeProgressColors(const EventStates&,
                                                              const Colors&);
   std::pair<sRGBColor, sRGBColor> ComputeRangeTrackColors(const EventStates&,
                                                           const Colors&);
   std::pair<sRGBColor, sRGBColor> ComputeRangeThumbColors(const EventStates&,
                                                           const Colors&);
   std::pair<sRGBColor, sRGBColor> ComputeProgressColors(const Colors&);
   std::pair<sRGBColor, sRGBColor> ComputeProgressTrackColors(const Colors&);