Issue #1970 - Part 2: Remove the extra padding on buttons

Extra padding was created for a prospective -moz-focus-inner ring.
We now just size that ring the same as the content frame, inflated by its CSS
padding.

layout/forms/nsButtonFrameRenderer.cpp

@@ -241,10 +241,9 @@ void nsDisplayButtonForeground::Paint(nsDisplayListBuilder* aBuilder,
       !presContext->GetTheme()->ThemeDrawsFocusForWidget(disp->mAppearance)) {
     nsRect r = nsRect(ToReferenceFrame(), mFrame->GetSize());
 
-    // Draw the focus and outline borders.
+    // Draw the -moz-focus-inner border
     DrawResult result =
-      mBFR->PaintOutlineAndFocusBorders(aBuilder, presContext, *aCtx,
+      mBFR->PaintInnerFocusBorder(aBuilder, presContext, *aCtx, mVisibleRect, r);
-                                        mVisibleRect, r);
 
     nsDisplayItemGenericImageGeometry::UpdateDrawResult(this, result);
   }
@@ -278,20 +277,29 @@ nsButtonFrameRenderer::DisplayButton(nsDisplayListBuilder* aBuilder,
   return NS_OK;
 }
 
+void
+nsButtonFrameRenderer::GetButtonInnerFocusRect(const nsRect& aRect, nsRect& aResult)
+{
+  GetButtonRect(aRect, aResult);
+  aResult.Deflate(mFrame->GetUsedBorderAndPadding());
+
+  nsMargin innerFocusPadding(0,0,0,0);
+  if (mInnerFocusStyle) {
+    mInnerFocusStyle->StylePadding()->GetPadding(innerFocusPadding);
+  }
+  aResult.Inflate(innerFocusPadding);
+}
+
 DrawResult
-nsButtonFrameRenderer::PaintOutlineAndFocusBorders(
+nsButtonFrameRenderer::PaintInnerFocusBorder(
   nsDisplayListBuilder* aBuilder,
   nsPresContext* aPresContext,
   nsRenderingContext& aRenderingContext,
   const nsRect& aDirtyRect,
   const nsRect& aRect)
 {
-  // once we have all that we'll draw the focus if we have it. We will
+  // we draw the -moz-focus-inner border just inside the button's
-  // need to draw 2 focuses, the inner and the outer. This is so we
+  // normal border and padding, to match Windows themes.
-  // can do any kind of look and feel. Some buttons have focus on the
-  // outside like mac and motif. While others like windows have it
-  // inside (dotted line).  Usually only one will be specifed. But I
-  // guess you could have both if you wanted to.
 
   nsRect rect;
 
@@ -348,57 +356,6 @@ nsButtonFrameRenderer::GetButtonRect(const nsRect& aRect, nsRect& r)
 }
 
 
-void
-nsButtonFrameRenderer::GetButtonInnerFocusRect(const nsRect& aRect, nsRect& focusRect)
-{
-  GetButtonRect(aRect, focusRect);
-  focusRect.Deflate(GetButtonBorderAndPadding());
-  focusRect.Deflate(GetButtonInnerFocusMargin());
-}
-
-
-nsMargin
-nsButtonFrameRenderer::GetButtonBorderAndPadding()
-{
-  return mFrame->GetUsedBorderAndPadding();
-}
-
-/**
- * Gets the size of the buttons border this is the union of the normal and disabled borders.
- */
-nsMargin
-nsButtonFrameRenderer::GetButtonInnerFocusMargin()
-{
-  nsMargin innerFocusMargin(0,0,0,0);
-
-  if (mInnerFocusStyle) {
-    const nsStyleMargin* margin = mInnerFocusStyle->StyleMargin();
-    margin->GetMargin(innerFocusMargin);
-  }
-
-  return innerFocusMargin;
-}
-
-nsMargin
-nsButtonFrameRenderer::GetButtonInnerFocusBorderAndPadding()
-{
-  nsMargin result(0,0,0,0);
-
-  if (mInnerFocusStyle) {
-    mInnerFocusStyle->StylePadding()->GetPadding(result);
-    result += mInnerFocusStyle->StyleBorder()->GetComputedBorder();
-  }
-
-  return result;
-}
-
-// gets all the focus borders and padding that will be added to the regular border
-nsMargin
-nsButtonFrameRenderer::GetAddedButtonBorderAndPadding()
-{
-  return GetButtonInnerFocusMargin() + GetButtonInnerFocusBorderAndPadding();
-}
-
 /**
  * Call this when styles change
  */
@@ -415,7 +372,7 @@ nsButtonFrameRenderer::ReResolveStyles(nsPresContext* aPresContext)
   }
 #endif
 
-  // style for the inner such as a dotted line (Windows)
+  // get styles assigned to -moz-inner-focus (ie dotted border on Windows)
   mInnerFocusStyle =
     styleSet->ProbePseudoElementStyle(mFrame->GetContent()->AsElement(),
                                       CSSPseudoElementType::mozFocusInner,

layout/forms/nsButtonFrameRenderer.h

@@ -37,11 +37,11 @@ public:
                          nsDisplayList* aBackground, nsDisplayList* aForeground);
 
 
-  DrawResult PaintOutlineAndFocusBorders(nsDisplayListBuilder* aBuilder,
+  DrawResult PaintInnerFocusBorder(nsDisplayListBuilder* aBuilder,
-                                         nsPresContext* aPresContext,
+                                   nsPresContext* aPresContext,
-                                         nsRenderingContext& aRenderingContext,
+                                   nsRenderingContext& aRenderingContext,
-                                         const nsRect& aDirtyRect,
+                                   const nsRect& aDirtyRect,
-                                         const nsRect& aRect);
+                                   const nsRect& aRect);
 
   DrawResult PaintBorder(nsDisplayListBuilder* aBuilder,
                          nsPresContext* aPresContext,
@@ -58,10 +58,6 @@ public:
 
   void GetButtonRect(const nsRect& aRect, nsRect& aResult);
   void GetButtonInnerFocusRect(const nsRect& aRect, nsRect& aResult);
-  nsMargin GetButtonBorderAndPadding();
-  nsMargin GetButtonInnerFocusMargin();
-  nsMargin GetButtonInnerFocusBorderAndPadding();
-  nsMargin GetAddedButtonBorderAndPadding();
 
   nsStyleContext* GetStyleContext(int32_t aIndex) const;
   void SetStyleContext(int32_t aIndex, nsStyleContext* aStyleContext);
@@ -73,7 +69,7 @@ protected:
 
 private:
 
-  // cached style for focus and outline (used on Windows).
+  // cached style for optional inner focus outline (used on Windows).
   RefPtr<nsStyleContext> mInnerFocusStyle;
 
   nsFrame* mFrame;

layout/forms/nsHTMLButtonControlFrame.cpp

@@ -157,10 +157,6 @@ nsHTMLButtonControlFrame::GetMinISize(nsRenderingContext* aRenderingContext)
                                                 kid,
                                                 nsLayoutUtils::MIN_ISIZE);
 
-  result += GetWritingMode().IsVertical()
-    ? mRenderer.GetAddedButtonBorderAndPadding().TopBottom()
-    : mRenderer.GetAddedButtonBorderAndPadding().LeftRight();
-
   return result;
 }
 
@@ -175,10 +171,6 @@ nsHTMLButtonControlFrame::GetPrefISize(nsRenderingContext* aRenderingContext)
                                                 kid,
                                                 nsLayoutUtils::PREF_ISIZE);
 
-  result += GetWritingMode().IsVertical()
-    ? mRenderer.GetAddedButtonBorderAndPadding().TopBottom()
-    : mRenderer.GetAddedButtonBorderAndPadding().LeftRight();
-
   return result;
 }
 
@@ -192,9 +184,6 @@ nsHTMLButtonControlFrame::Reflow(nsPresContext* aPresContext,
   DO_GLOBAL_REFLOW_COUNT("nsHTMLButtonControlFrame");
   DISPLAY_REFLOW(aPresContext, this, aReflowInput, aDesiredSize, aStatus);
 
-  NS_PRECONDITION(aReflowInput.ComputedISize() != NS_INTRINSICSIZE,
-                  "Should have real computed inline-size by now");
-
   if (mState & NS_FRAME_FIRST_REFLOW) {
     nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), true);
   }
@@ -237,34 +226,6 @@ nsHTMLButtonControlFrame::Reflow(nsPresContext* aPresContext,
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize);
 }
 
-// Helper-function that lets us clone the button's reflow state, but with its
-// ComputedWidth and ComputedHeight reduced by the amount of renderer-specific
-// focus border and padding that we're using. (This lets us provide a more
-// appropriate content-box size for descendents' percent sizes to resolve
-// against.)
-static ReflowInput
-CloneReflowInputWithReducedContentBox(
-  const ReflowInput& aButtonReflowInput,
-  const nsMargin& aFocusPadding)
-{
-  nscoord adjustedWidth =
-    aButtonReflowInput.ComputedWidth() - aFocusPadding.LeftRight();
-  adjustedWidth = std::max(0, adjustedWidth);
-
-  // (Only adjust height if it's an actual length.)
-  nscoord adjustedHeight = aButtonReflowInput.ComputedHeight();
-  if (adjustedHeight != NS_INTRINSICSIZE) {
-    adjustedHeight -= aFocusPadding.TopBottom();
-    adjustedHeight = std::max(0, adjustedHeight);
-  }
-
-  ReflowInput clone(aButtonReflowInput);
-  clone.SetComputedWidth(adjustedWidth);
-  clone.SetComputedHeight(adjustedHeight);
-
-  return clone;
-}
-
 void
 nsHTMLButtonControlFrame::ReflowButtonContents(nsPresContext* aPresContext,
                                                ReflowOutput& aButtonDesiredSize,
@@ -275,53 +236,17 @@ nsHTMLButtonControlFrame::ReflowButtonContents(nsPresContext* aPresContext,
   LogicalSize availSize = aButtonReflowInput.ComputedSize(wm);
   availSize.BSize(wm) = NS_INTRINSICSIZE;
 
-  // Buttons have some bonus renderer-determined border/padding,
-  // which occupies part of the button's content-box area:
-  LogicalMargin focusPadding =
-    LogicalMargin(wm, mRenderer.GetAddedButtonBorderAndPadding());
-
-  // See whether out availSize's inline-size is big enough.  If it's
-  // smaller than our intrinsic min iSize, that means that the kid
-  // wouldn't really fit.  In that case, we overflow into our internal
-  // focuspadding (which other browsers don't have) so that there's a
-  // little more space for it.
-  // Note that GetMinISize includes the focusPadding.
-  nscoord IOverflow = GetMinISize(aButtonReflowInput.mRenderingContext) -
-                      aButtonReflowInput.ComputedISize();
-  nscoord IFocusPadding = focusPadding.IStartEnd(wm);
-  nscoord focusPaddingReduction = std::min(IFocusPadding,
-                                           std::max(IOverflow, 0));
-  if (focusPaddingReduction > 0) {
-    nscoord startReduction = focusPadding.IStart(wm);
-    if (focusPaddingReduction != IFocusPadding) {
-      startReduction = NSToCoordRound(startReduction *
-                                      (float(focusPaddingReduction) /
-                                       float(IFocusPadding)));
-    }
-    focusPadding.IStart(wm) -= startReduction;
-    focusPadding.IEnd(wm) -= focusPaddingReduction - startReduction;
-  }
-
   // shorthand for a value we need to use in a bunch of places
   const LogicalMargin& clbp = aButtonReflowInput.ComputedLogicalBorderPadding();
 
-  // Indent the child inside us by the focus border. We must do this separate
-  // from the regular border.
-  availSize.ISize(wm) -= focusPadding.IStartEnd(wm);
-
   LogicalPoint childPos(wm);
-  childPos.I(wm) = focusPadding.IStart(wm) + clbp.IStart(wm);
+  childPos.I(wm) = clbp.IStart(wm);
   availSize.ISize(wm) = std::max(availSize.ISize(wm), 0);
 
-  // Give child a clone of the button's reflow state, with height/width reduced
-  // by focusPadding, so that descendants with height:100% don't protrude.
-  ReflowInput adjustedButtonReflowInput =
-    CloneReflowInputWithReducedContentBox(aButtonReflowInput,
-                                          focusPadding.GetPhysicalMargin(wm));
-
   ReflowInput contentsReflowInput(aPresContext,
-                                        adjustedButtonReflowInput,
+                                  aButtonReflowInput,
-                                        aFirstKid, availSize);
+                                  aFirstKid,
+                                  availSize);
 
   nsReflowStatus contentsReflowStatus;
   ReflowOutput contentsDesiredSize(aButtonReflowInput);
@@ -346,9 +271,8 @@ nsHTMLButtonControlFrame::ReflowButtonContents(nsPresContext* aPresContext,
     buttonContentBox.BSize(wm) = aButtonReflowInput.ComputedBSize();
   } else {
     // Button is intrinsically sized -- it should shrinkwrap the
-    // button-contents' bSize, plus any focus-padding space:
+    // button-contents' bSize:
-    buttonContentBox.BSize(wm) =
+    buttonContentBox.BSize(wm) = contentsDesiredSize.BSize(wm);
-      contentsDesiredSize.BSize(wm) + focusPadding.BStartEnd(wm);
 
     // Make sure we obey min/max-bSize in the case when we're doing intrinsic
     // sizing (we get it for free when we have a non-intrinsic
@@ -363,8 +287,7 @@ nsHTMLButtonControlFrame::ReflowButtonContents(nsPresContext* aPresContext,
   if (aButtonReflowInput.ComputedISize() != NS_INTRINSICSIZE) {
     buttonContentBox.ISize(wm) = aButtonReflowInput.ComputedISize();
   } else {
-    buttonContentBox.ISize(wm) =
+    buttonContentBox.ISize(wm) = contentsDesiredSize.ISize(wm);
-      contentsDesiredSize.ISize(wm) + focusPadding.IStartEnd(wm);
     buttonContentBox.ISize(wm) =
       NS_CSS_MINMAX(buttonContentBox.ISize(wm),
                     aButtonReflowInput.ComputedMinISize(),
@@ -372,16 +295,12 @@ nsHTMLButtonControlFrame::ReflowButtonContents(nsPresContext* aPresContext,
   }
 
   // Center child in the block-direction in the button
-  // (technically, inside of the button's focus-padding area)
+  nscoord extraSpace = buttonContentBox.BSize(wm) - contentsDesiredSize.BSize(wm);
-  nscoord extraSpace =
-    buttonContentBox.BSize(wm) - focusPadding.BStartEnd(wm) -
-    contentsDesiredSize.BSize(wm);
 
   childPos.B(wm) = std::max(0, extraSpace / 2);
 
-  // Adjust childPos.B() to be in terms of the button's frame-rect, instead of
+  // Adjust childPos.B() to be in terms of the button's frame-rect:
-  // its focus-padding rect:
+  childPos.B(wm) += clbp.BStart(wm);
-  childPos.B(wm) += focusPadding.BStart(wm) + clbp.BStart(wm);
 
   nsSize containerSize =
     (buttonContentBox + clbp.Size(wm)).GetPhysicalSize(wm);

layout/style/nsLayoutStylesheetCache.cpp

@@ -637,7 +637,6 @@ nsLayoutStylesheetCache::BuildPreferenceSheet(RefPtr<StyleSheet>* aSheet,
           "button::-moz-focus-inner, input[type=\"reset\"]::-moz-focus-inner, "
           "input[type=\"button\"]::-moz-focus-inner, "
           "input[type=\"submit\"]::-moz-focus-inner { "
-          "padding: 1px 2px 1px 2px; "
           "border: %dpx %s transparent !important; }\n",
           focusRingWidth,
           focusRingStyle == 0 ? "solid" : "dotted");

layout/style/res/forms.css

@@ -627,12 +627,12 @@ input[type="button"],
 input[type="submit"] {
   -moz-appearance: button;
   /* The sum of border and padding on block-start and block-end
-     must be the same here, for text inputs, and for <select>.  For
+     must be the same here, for text inputs, and for <select>.
-     buttons, make sure to include the -moz-focus-inner border/padding. */
+     Note -moz-focus-inner padding does not affect button size. */
   padding-block-start: 0px;
-  padding-inline-end: 6px;
+  padding-inline-end: 8px;
   padding-block-end: 0px;
-  padding-inline-start: 6px;
+  padding-inline-start: 8px;
   border: 2px outset ThreeDLightShadow;
   background-color: ButtonFace;
   cursor: default;
@@ -725,9 +725,9 @@ input[type="button"]:active:hover,
 input[type="submit"]:active:hover {
 %ifndef XP_MACOSX
   padding-block-start: 0px;
-  padding-inline-end: 5px;
+  padding-inline-end: 7px;
   padding-block-end: 0px;
-  padding-inline-start: 7px;
+  padding-inline-start: 9px;
 %endif
   border-style: inset;
   background-color: ButtonFace;
@@ -746,6 +746,7 @@ input[type="reset"]::-moz-focus-inner,
 input[type="button"]::-moz-focus-inner,
 input[type="submit"]::-moz-focus-inner,
 input[type="file"] > button[type="button"]::-moz-focus-inner {
+  /* Note this padding only affects the -moz-focus-inner ring, not the button itself */
   padding-block-start: 0px;
   padding-inline-end: 2px;
   padding-block-end: 0px;
@@ -776,9 +777,9 @@ input[type="submit"]:disabled {
   /* The sum of border and padding on block-start and block-end
      must be the same here and for text inputs */
   padding-block-start: 0px;
-  padding-inline-end: 6px;
+  padding-inline-end: 8px;
   padding-block-end: 0px;
-  padding-inline-start: 6px;
+  padding-inline-start: 8px;
   border: 2px outset ThreeDLightShadow;
   cursor: inherit;
 }

widget/gtk/gtk3drawing.cpp

@@ -2036,9 +2036,6 @@ moz_gtk_get_widget_border(WidgetNodeType widget, gint* left, gint* top,
             if (widget == MOZ_GTK_TOOLBAR_BUTTON)
                 gtk_style_context_restore(style);
 
-            // XXX: Subtract 1 pixel from the border to account for the added
-            // -moz-focus-inner border (Bug 1228281).
-            *left -= 1; *top -= 1; *right -= 1; *bottom -= 1;
             moz_gtk_add_style_border(style, left, top, right, bottom);
 
             ReleaseStyleContext(style);