From 3d954cb29cce81e033a6c216ac29138e5fe7b571 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 22 Jul 2022 15:06:19 +0000 Subject: 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 | 81 ++++++------------------ layout/forms/nsButtonFrameRenderer.h | 16 ++--- layout/forms/nsHTMLButtonControlFrame.cpp | 101 +++--------------------------- layout/style/nsLayoutStylesheetCache.cpp | 1 - layout/style/res/forms.css | 17 ++--- widget/gtk/gtk3drawing.cpp | 3 - 6 files changed, 44 insertions(+), 175 deletions(-) diff --git a/layout/forms/nsButtonFrameRenderer.cpp b/layout/forms/nsButtonFrameRenderer.cpp index bf513374a0..eaf1f82c14 100644 --- a/layout/forms/nsButtonFrameRenderer.cpp +++ b/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, - mVisibleRect, r); + mBFR->PaintInnerFocusBorder(aBuilder, presContext, *aCtx, 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 - // need to draw 2 focuses, the inner and the outer. This is so we - // 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. + // we draw the -moz-focus-inner border just inside the button's + // normal border and padding, to match Windows themes. 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, diff --git a/layout/forms/nsButtonFrameRenderer.h b/layout/forms/nsButtonFrameRenderer.h index 6557bc0927..d21303efaf 100644 --- a/layout/forms/nsButtonFrameRenderer.h +++ b/layout/forms/nsButtonFrameRenderer.h @@ -37,11 +37,11 @@ public: nsDisplayList* aBackground, nsDisplayList* aForeground); - DrawResult PaintOutlineAndFocusBorders(nsDisplayListBuilder* aBuilder, - nsPresContext* aPresContext, - nsRenderingContext& aRenderingContext, - const nsRect& aDirtyRect, - const nsRect& aRect); + DrawResult PaintInnerFocusBorder(nsDisplayListBuilder* aBuilder, + nsPresContext* aPresContext, + nsRenderingContext& aRenderingContext, + const nsRect& aDirtyRect, + 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 mInnerFocusStyle; nsFrame* mFrame; diff --git a/layout/forms/nsHTMLButtonControlFrame.cpp b/layout/forms/nsHTMLButtonControlFrame.cpp index afedfa7748..1ec0ec6c94 100644 --- a/layout/forms/nsHTMLButtonControlFrame.cpp +++ b/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(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, - aFirstKid, availSize); + aButtonReflowInput, + 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: - buttonContentBox.BSize(wm) = - contentsDesiredSize.BSize(wm) + focusPadding.BStartEnd(wm); + // button-contents' bSize: + buttonContentBox.BSize(wm) = contentsDesiredSize.BSize(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) = - contentsDesiredSize.ISize(wm) + focusPadding.IStartEnd(wm); + buttonContentBox.ISize(wm) = contentsDesiredSize.ISize(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) - focusPadding.BStartEnd(wm) - - contentsDesiredSize.BSize(wm); + nscoord extraSpace = buttonContentBox.BSize(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 - // its focus-padding rect: - childPos.B(wm) += focusPadding.BStart(wm) + clbp.BStart(wm); + // Adjust childPos.B() to be in terms of the button's frame-rect: + childPos.B(wm) += clbp.BStart(wm); nsSize containerSize = (buttonContentBox + clbp.Size(wm)).GetPhysicalSize(wm); diff --git a/layout/style/nsLayoutStylesheetCache.cpp b/layout/style/nsLayoutStylesheetCache.cpp index 7c09202e6a..9591ad4f4a 100644 --- a/layout/style/nsLayoutStylesheetCache.cpp +++ b/layout/style/nsLayoutStylesheetCache.cpp @@ -637,7 +637,6 @@ nsLayoutStylesheetCache::BuildPreferenceSheet(RefPtr* 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"); diff --git a/layout/style/res/forms.css b/layout/style/res/forms.css index db75151d48..b0b171a892 100644 --- a/layout/style/res/forms.css +++ b/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 . + 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; } diff --git a/widget/gtk/gtk3drawing.cpp b/widget/gtk/gtk3drawing.cpp index c592fa51c9..c3216fbf34 100644 --- a/widget/gtk/gtk3drawing.cpp +++ b/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); -- cgit v1.2.3