Backed out changeset f5546bfc9604 (bug 1636998) for reftest failures …

…on 1174332-1.html . CLOSED TREE

gfx/src/nsITheme.h

@@ -213,17 +213,6 @@ class nsITheme : public nsISupports {
    */
   virtual bool ThemeDrawsFocusForWidget(StyleAppearance aWidgetType) = 0;
 
-  /**
-   * Whether we want an inner focus ring for buttons and such.
-   *
-   * Usually, we don't want it if we have our own focus indicators, but windows
-   * is special, because it wants it even though focus also alters the border
-   * color and such.
-   */
-  virtual bool ThemeWantsButtonInnerFocusRing(StyleAppearance aAppearance) {
-    return !ThemeDrawsFocusForWidget(aAppearance);
-  }
-
   /**
    * Should we insert a dropmarker inside of combobox button?
    */

layout/forms/nsButtonFrameRenderer.cpp

@@ -347,7 +347,7 @@ void nsDisplayButtonForeground::Paint(nsDisplayListBuilder* aBuilder,
   nsPresContext* presContext = mFrame->PresContext();
   const nsStyleDisplay* disp = mFrame->StyleDisplay();
   if (!mFrame->IsThemed(disp) ||
-      presContext->Theme()->ThemeWantsButtonInnerFocusRing(disp->mAppearance)) {
+      !presContext->Theme()->ThemeDrawsFocusForWidget(disp->mAppearance)) {
     nsRect r = nsRect(ToReferenceFrame(), mFrame->GetSize());
 
     // Draw the -moz-focus-inner border
@@ -369,7 +369,7 @@ bool nsDisplayButtonForeground::CreateWebRenderCommands(
   nsPresContext* presContext = mFrame->PresContext();
   const nsStyleDisplay* disp = mFrame->StyleDisplay();
   if (!mFrame->IsThemed(disp) ||
-      presContext->Theme()->ThemeWantsButtonInnerFocusRing(disp->mAppearance)) {
+      !presContext->Theme()->ThemeDrawsFocusForWidget(disp->mAppearance)) {
     nsRect r = nsRect(ToReferenceFrame(), mFrame->GetSize());
     br = mBFR->CreateInnerFocusBorderRenderer(aDisplayListBuilder, presContext,
                                               nullptr, GetPaintRect(), r,

layout/forms/nsComboboxControlFrame.cpp

@@ -1504,9 +1504,8 @@ void nsComboboxControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
     if (window && window->ShouldShowFocusRing()) {
       nsPresContext* presContext = PresContext();
       const nsStyleDisplay* disp = StyleDisplay();
-      if ((!IsThemed(disp) ||
-           presContext->Theme()->ThemeWantsButtonInnerFocusRing(
-               disp->mAppearance)) &&
+      if ((!IsThemed(disp) || !presContext->Theme()->ThemeDrawsFocusForWidget(
+                                  disp->mAppearance)) &&
           mDisplayFrame && IsVisibleForPainting()) {
         aLists.Content()->AppendNewToTop<nsDisplayComboboxFocus>(aBuilder,
                                                                  this);

layout/forms/nsRangeFrame.cpp

@@ -261,11 +261,8 @@ void nsRangeFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
     return;
   }
 
-  // FIXME(emilio): This is using ThemeWantsButtonInnerFocusRing even though
-  // it's painting the ::-moz-focus-outer pseudo-class... But why is
-  // ::-moz-focus-outer useful, instead of outline?
-  if (IsThemed(disp) && !PresContext()->Theme()->ThemeWantsButtonInnerFocusRing(
-                            disp->mAppearance)) {
+  if (IsThemed(disp) &&
+      PresContext()->Theme()->ThemeDrawsFocusForWidget(disp->mAppearance)) {
     return;  // the native theme displays its own visual indication of focus
   }
 

layout/generic/nsFrame.cpp

@@ -2625,18 +2625,12 @@ void nsFrame::DisplayOutlineUnconditional(nsDisplayListBuilder* aBuilder,
     return;
   }
 
-  // We don't display outline-style: auto on themed frames that have their own
-  // focus indicators.
-  if (outline.mOutlineStyle.IsAuto()) {
-    auto* disp = StyleDisplay();
-    // FIXME(emilio): The range special-case is needed because <input
-    // type=range> displays its own outline with ::-moz-focus-outer, and this
-    // would show two outlines instead of one.
-    if (IsThemed(disp) &&
-        (PresContext()->Theme()->ThemeDrawsFocusForWidget(disp->mAppearance) ||
-         disp->mAppearance != StyleAppearance::Range)) {
-      return;
-    }
+  // We don't display outline-style: auto on themed frames.
+  //
+  // TODO(emilio): Maybe we want a theme hook to say which frames can handle it
+  // themselves. Non-native theme probably will want this.
+  if (outline.mOutlineStyle.IsAuto() && IsThemed()) {
+    return;
   }
 
   aLists.Outlines()->AppendNewToTop<nsDisplayOutline>(aBuilder, this);

layout/style/res/forms.css

@@ -470,6 +470,10 @@ input[type="image"]:disabled {
   cursor: unset;
 }
 
+input[type="image"]:-moz-focusring {
+  /* Don't specify the outline-color, we should always use initial value. */
+  outline: 1px dotted;
+}
 
 /* file selector */
 input[type="file"] {
@@ -558,15 +562,15 @@ input[type="checkbox"]:disabled:hover:active {
   cursor: unset;
 }
 
-input:not([type="file"]):not([type="image"]):-moz-focusring,
-select:-moz-focusring,
-button:-moz-focusring,
-textarea:-moz-focusring {
-  /* These elements can handle outline themselves when themed, so we use
-   * outline-style: auto and skip rendering the outline only when themed and
-   * the theme allows so */
-  outline-style: auto;
+% On Mac, the native theme takes care of this.
+% See nsNativeThemeCocoa::ThemeDrawsFocusForWidget.
+%ifndef XP_MACOSX
+input[type="checkbox"]:-moz-focusring,
+input[type="radio"]:-moz-focusring {
+  /* Don't specify the outline-color, we should always use initial value. */
+  outline: 1px dotted;
 }
+%endif
 
 input[type="search"] {
   box-sizing: border-box;

layout/style/res/html.css

@@ -726,6 +726,16 @@ html:-moz-focusring {
   outline-style: none;
 }
 
+input:not([type="file"]):-moz-focusring,
+button:-moz-focusring,
+select:-moz-focusring,
+button:-moz-focusring,
+textarea:-moz-focusring {
+  /* These elements can handle outline themselves when themed, so we use
+   * outline-style: auto and skip rendering the outline only when themed */
+  outline-style: auto;
+}
+
 /* hidden elements */
 base, basefont, datalist, head, meta, script, style, title,
 noembed, param, template {

widget/cocoa/nsNativeThemeCocoa.mm

@@ -4204,24 +4204,15 @@ static bool IsHiDPIContext(nsDeviceContext* aContext) {
 }
 
 bool nsNativeThemeCocoa::ThemeDrawsFocusForWidget(StyleAppearance aAppearance) {
-  switch (aAppearance) {
-    case StyleAppearance::Textarea:
-    case StyleAppearance::Textfield:
-    case StyleAppearance::Searchfield:
-    case StyleAppearance::NumberInput:
-    case StyleAppearance::Menulist:
-    case StyleAppearance::MenulistButton:
-    case StyleAppearance::Button:
-    case StyleAppearance::MozMacHelpButton:
-    case StyleAppearance::MozMacDisclosureButtonOpen:
-    case StyleAppearance::MozMacDisclosureButtonClosed:
-    case StyleAppearance::Radio:
-    case StyleAppearance::Range:
-    case StyleAppearance::Checkbox:
-      return true;
-    default:
-      return false;
-  }
+  if (aAppearance == StyleAppearance::MenulistButton || aAppearance == StyleAppearance::Button ||
+      aAppearance == StyleAppearance::MozMacHelpButton ||
+      aAppearance == StyleAppearance::MozMacDisclosureButtonOpen ||
+      aAppearance == StyleAppearance::MozMacDisclosureButtonClosed ||
+      aAppearance == StyleAppearance::Radio || aAppearance == StyleAppearance::Range ||
+      aAppearance == StyleAppearance::Checkbox)
+    return true;
+
+  return false;
 }
 
 bool nsNativeThemeCocoa::ThemeNeedsComboboxDropmarker() { return false; }

widget/gtk/nsNativeThemeGTK.cpp

@@ -1972,19 +1972,13 @@ nsNativeThemeGTK::WidgetIsContainer(StyleAppearance aAppearance) {
 }
 
 bool nsNativeThemeGTK::ThemeDrawsFocusForWidget(StyleAppearance aAppearance) {
-  switch (aAppearance) {
-    case StyleAppearance::Button:
-    case StyleAppearance::Menulist:
-    case StyleAppearance::MenulistButton:
-    case StyleAppearance::MenulistTextfield:
-    case StyleAppearance::Textarea:
-    case StyleAppearance::Textfield:
-    case StyleAppearance::Treeheadercell:
-    case StyleAppearance::NumberInput:
-      return true;
-    default:
-      return false;
-  }
+  if (aAppearance == StyleAppearance::Menulist ||
+      aAppearance == StyleAppearance::MenulistButton ||
+      aAppearance == StyleAppearance::Button ||
+      aAppearance == StyleAppearance::Treeheadercell)
+    return true;
+
+  return false;
 }
 
 bool nsNativeThemeGTK::ThemeNeedsComboboxDropmarker() { return false; }

widget/nsNativeBasicTheme.cpp

@@ -910,15 +910,7 @@ bool nsNativeBasicTheme::WidgetIsContainer(StyleAppearance aAppearance) {
 }
 
 bool nsNativeBasicTheme::ThemeDrawsFocusForWidget(StyleAppearance aAppearance) {
-  switch (aAppearance) {
-    // TODO(emilio): Checkbox / Radio don't have focus indicators when checked.
-    // If they did, we could just return true here unconditionally.
-    case StyleAppearance::Checkbox:
-    case StyleAppearance::Radio:
-      return false;
-    default:
-      return true;
-  }
+  return true;
 }
 
 bool nsNativeBasicTheme::ThemeNeedsComboboxDropmarker() { return true; }

widget/windows/nsNativeThemeWin.cpp

@@ -2601,17 +2601,7 @@ bool nsNativeThemeWin::WidgetIsContainer(StyleAppearance aAppearance) {
 }
 
 bool nsNativeThemeWin::ThemeDrawsFocusForWidget(StyleAppearance aAppearance) {
-  switch (aAppearance) {
-    case StyleAppearance::Menulist:
-    case StyleAppearance::MenulistButton:
-    case StyleAppearance::MenulistTextfield:
-    case StyleAppearance::Textarea:
-    case StyleAppearance::Textfield:
-    case StyleAppearance::NumberInput:
-      return true;
-    default:
-      return false;
-  }
+  return false;
 }
 
 bool nsNativeThemeWin::ThemeNeedsComboboxDropmarker() { return true; }

widget/windows/nsNativeThemeWin.h

@@ -68,8 +68,6 @@ class nsNativeThemeWin : private nsNativeTheme, public nsITheme {
 
   bool ThemeDrawsFocusForWidget(StyleAppearance aAppearance) override;
 
-  bool ThemeWantsButtonInnerFocusRing(StyleAppearance) override { return true; }
-
   bool ThemeNeedsComboboxDropmarker() override;
 
   virtual bool WidgetAppearanceDependsOnWindowFocus(