Skip to content

Commit

Permalink
Bug 1693688 - Make the non-native theme not return minimum sizes for …
Browse files Browse the repository at this point in the history
…checkboxes (and most other widgets). r=spohl,mstange

This matches closer what Chrome and Safari do (Safari paints outside of
the box when this happens, but the layout box still respects the
author), see:

  data:text/html,<button style="padding: 0; width: 0">
  data:text/html,<input type=checkbox style="width: 0">

Etc. For checkboxes, this matches what OSX does, too.

Since we still want checkboxes to be slightly larger than what they'd be
otherwise, we add a hook to tweak it when non-native theme is enabled.

Differential Revision: https://phabricator.services.mozilla.com/D105798
  • Loading branch information
emilio committed Feb 20, 2021
1 parent f108651 commit 6825bbc
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 30 deletions.
5 changes: 3 additions & 2 deletions devtools/client/shared/test/doc_options-view.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
label="black box"/>
</menupopup>
</popupset>
<button id="options-button"
popup="options-menupopup"/>
<button id="options-button" popup="options-menupopup">
Options
</button>
</window>
8 changes: 8 additions & 0 deletions gfx/src/nsITheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@ class nsITheme : public nsISupports {
return false;
}

/**
* Get the preferred content-box size of a checkbox / radio button, in app
* units. Historically 9px.
*/
virtual nscoord GetCheckboxRadioPrefSize() {
return mozilla::CSSPixel::ToAppUnits(9);
}

/**
* Get the minimum border-box size of a widget, in *pixels* (in
* |aResult|). If |aIsOverridable| is set to true, this size is a
Expand Down
8 changes: 7 additions & 1 deletion layout/forms/nsCheckboxRadioFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "nsStyleConsts.h"

using namespace mozilla;
using mozilla::dom::Element;
using mozilla::dom::HTMLInputElement;

//#define FCF_NOISY
Expand All @@ -37,6 +36,13 @@ NS_QUERYFRAME_HEAD(nsCheckboxRadioFrame)
NS_QUERYFRAME_ENTRY(nsIFormControlFrame)
NS_QUERYFRAME_TAIL_INHERITING(nsAtomicContainerFrame)

nscoord nsCheckboxRadioFrame::DefaultSize() {
if (StyleDisplay()->HasAppearance()) {
return PresContext()->Theme()->GetCheckboxRadioPrefSize();
}
return CSSPixel::ToAppUnits(9);
}

/* virtual */
nscoord nsCheckboxRadioFrame::GetMinISize(gfxContext* aRenderingContext) {
nscoord result;
Expand Down
7 changes: 1 addition & 6 deletions layout/forms/nsCheckboxRadioFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,7 @@ class nsCheckboxRadioFrame final : public nsAtomicContainerFrame,
protected:
virtual ~nsCheckboxRadioFrame();

static nscoord DefaultSize() {
// XXXmats We have traditionally always returned 9px for GetMin/PrefISize
// but we might want to factor in what the theme says, something like:
// GetMinimumWidgetSize - GetWidgetPadding - GetWidgetBorder.
return nsPresContext::CSSPixelsToAppUnits(9);
}
nscoord DefaultSize();

/**
* Get the state of the checked attribute.
Expand Down
4 changes: 2 additions & 2 deletions layout/reftests/forms/input/checkbox/reftest.list
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
!= indeterminate-native-checked.html indeterminate-native-checked-notref.html
!= indeterminate-native-unchecked.html indeterminate-native-unchecked-notref.html
== indeterminate-selector.html indeterminate-selector-ref.html
skip-if(!gtkWidget) == gtk-theme-width-height.html gtk-theme-width-height-ref.html
skip-if(!gtkWidget||!nativeThemePref) == gtk-theme-width-height.html gtk-theme-width-height-ref.html
== checkbox-baseline.html checkbox-baseline-ref.html
== checkbox-radio-color.html checkbox-radio-color-ref.html
skip-if(gtkWidget&&nativeThemePref) == checkbox-clamp-01.html checkbox-clamp-01-ref.html
skip-if(OSX||winWidget) fails-if(geckoview&&webrender) fuzzy-if(gtkWidget&&nativeThemePref,25-25,32-32) fails-if(Android&&nativeThemePref) == checkbox-clamp-02.html checkbox-clamp-02-ref.html
fails-if(OSX&&nativeThemePref) == checkbox-minimum-size.html checkbox-minimum-size-ref.html
fails-if(!OSX&&nativeThemePref) != checkbox-minimum-size.html checkbox-minimum-size-notref.html
4 changes: 2 additions & 2 deletions layout/reftests/forms/input/radio/reftest.list
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
== unchecked-appearance-none.html about:blank
!= checked-native.html about:blank
!= checked-native-notref.html about:blank
skip-if(!gtkWidget) == gtk-theme-width-height.html gtk-theme-width-height-ref.html
skip-if(!gtkWidget||!nativeThemePref) == gtk-theme-width-height.html gtk-theme-width-height-ref.html
skip-if(gtkWidget&&nativeThemePref) == radio-clamp-01.html radio-clamp-01-ref.html
skip-if(OSX||winWidget||Android) fuzzy-if(gtkWidget&&nativeThemePref,24-24,16-16) == radio-clamp-02.html radio-clamp-02-ref.html # gtkWidget, Bug 1599622
fails-if(OSX&&nativeThemePref) == radio-minimum-size.html radio-minimum-size-ref.html
fails-if(!OSX&&nativeThemePref) != radio-minimum-size.html radio-minimum-size-notref.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@
.v-rl { writing-mode: vertical-rl; }
.ltr, .rtl, .v-rl { border: 1px solid blue; }

input[type="radio"],
input[type="checkbox"] {
width: 13px;
height: 13px;
}

.ltr input[type="radio"] {
margin: 3px 3px 0px 5px;
}
Expand Down
27 changes: 17 additions & 10 deletions widget/nsNativeBasicTheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,12 @@ void nsNativeBasicTheme::PaintCheckboxControl(DrawTarget* aDrawTarget,
}
}

// Returns the right scale to cover aRect in the smaller dimension.
static float ScaleToWidgetRect(const LayoutDeviceRect& aRect) {
return std::min(aRect.width, aRect.height) / kMinimumWidgetSize;

// Returns the right scale for points in a 14x14 unit box centered at 0x0 to
// fill aRect in the smaller dimension.
static float ScaleToFillRect(const LayoutDeviceRect& aRect) {
static constexpr float kPathPointsScale = 14.0f;
return std::min(aRect.width, aRect.height) / kPathPointsScale;
}

void nsNativeBasicTheme::PaintCheckMark(DrawTarget* aDrawTarget,
Expand All @@ -669,7 +672,7 @@ void nsNativeBasicTheme::PaintCheckMark(DrawTarget* aDrawTarget,
const float checkPolygonY[] = {0.5f, 4.0f, 4.0f, -2.5f, -4.0f,
-4.0f, 1.0f, 1.25f, -1.0f};
const int32_t checkNumPoints = sizeof(checkPolygonX) / sizeof(float);
const float scale = ScaleToWidgetRect(aRect);
const float scale = ScaleToFillRect(aRect);
auto center = aRect.Center().ToUnknownPoint();

RefPtr<PathBuilder> builder = aDrawTarget->CreatePathBuilder();
Expand All @@ -689,7 +692,7 @@ void nsNativeBasicTheme::PaintIndeterminateMark(DrawTarget* aDrawTarget,
const LayoutDeviceRect& aRect,
const EventStates& aState) {
const CSSCoord borderWidth = 2.0f;
const float scale = ScaleToWidgetRect(aRect);
const float scale = ScaleToFillRect(aRect);

Rect rect = aRect.ToUnknownRect();
rect.y += (rect.height / 2) - (borderWidth * scale / 2);
Expand Down Expand Up @@ -788,7 +791,7 @@ void nsNativeBasicTheme::PaintRadioCheckmark(DrawTarget* aDrawTarget,
const EventStates& aState,
DPIRatio aDpiRatio) {
const CSSCoord borderWidth = 2.0f;
const float scale = ScaleToWidgetRect(aRect);
const float scale = ScaleToFillRect(aRect);
auto [backgroundColor, checkColor] = ComputeRadioCheckmarkColors(aState);

LayoutDeviceRect rect(aRect);
Expand Down Expand Up @@ -853,7 +856,7 @@ void nsNativeBasicTheme::PaintArrow(DrawTarget* aDrawTarget,
const float aArrowPolygonY[],
const int32_t aArrowNumPoints,
const sRGBColor aFillColor) {
const float scale = ScaleToWidgetRect(aRect);
const float scale = ScaleToFillRect(aRect);

auto center = aRect.Center().ToUnknownPoint();

Expand Down Expand Up @@ -900,7 +903,7 @@ void nsNativeBasicTheme::PaintSpinnerButton(nsIFrame* aFrame,
const float arrowPolygonY[] = {-1.875f, 2.625f, 2.625f, -1.875f, -4.125f,
-4.125f, 0.375f, 0.375f, -4.125f, -4.125f};
const int32_t arrowNumPoints = ArrayLength(arrowPolygonX);
const float scaleX = ScaleToWidgetRect(aRect);
const float scaleX = ScaleToFillRect(aRect);
const float scaleY =
aAppearance == StyleAppearance::SpinnerDownbutton ? scaleX : -scaleX;

Expand Down Expand Up @@ -1546,6 +1549,10 @@ auto nsNativeBasicTheme::GetScrollbarSizes(nsPresContext* aPresContext,
return {s, s};
}

nscoord nsNativeBasicTheme::GetCheckboxRadioPrefSize() {
return CSSPixel::ToAppUnits(10);
}

NS_IMETHODIMP
nsNativeBasicTheme::GetMinimumWidgetSize(nsPresContext* aPresContext,
nsIFrame* aFrame,
Expand All @@ -1554,7 +1561,8 @@ nsNativeBasicTheme::GetMinimumWidgetSize(nsPresContext* aPresContext,
bool* aIsOverridable) {
DPIRatio dpiRatio = GetDPIRatio(aFrame, aAppearance);

aResult->width = aResult->height = (kMinimumWidgetSize * dpiRatio).Rounded();
aResult->width = aResult->height = 0;
*aIsOverridable = true;

switch (aAppearance) {
case StyleAppearance::Button:
Expand Down Expand Up @@ -1621,7 +1629,6 @@ nsNativeBasicTheme::GetMinimumWidgetSize(nsPresContext* aPresContext,
break;
}

*aIsOverridable = true;
return NS_OK;
}

Expand Down
3 changes: 2 additions & 1 deletion widget/nsNativeBasicTheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ static const gfx::sRGBColor sScrollbarButtonHoverColor(gfx::sRGBColor(0.86f,
0.86f,
0.86f));

static const CSSCoord kMinimumWidgetSize = 14.0f;
static const CSSCoord kMinimumScrollbarSize = 17.0f;
static const CSSCoord kMinimumThinScrollbarSize = 6.0f;
static const CSSCoord kMinimumColorPickerHeight = 32.0f;
Expand Down Expand Up @@ -175,6 +174,8 @@ class nsNativeBasicTheme : protected nsNativeTheme, public nsITheme {
Overlay) override;
static nscolor AdjustUnthemedScrollbarThumbColor(nscolor, EventStates);

nscoord GetCheckboxRadioPrefSize() override;

protected:
nsNativeBasicTheme() = default;
virtual ~nsNativeBasicTheme() = default;
Expand Down

0 comments on commit 6825bbc

Please sign in to comment.