Skip to content

Commit

Permalink
Bug 1860386 - Use non-native drawing for -moz-menulist-arrow-button. …
Browse files Browse the repository at this point in the history 
…r=stransky

The current code is unused, because we hide the combobox dropmarker in:

 * https://searchfox.org/mozilla-central/rev/cc35ffeaf33a3f4e0b2ce6b77f9e5817a705e0c8/toolkit/themes/linux/global/menulist.css#46

If you remove that rule, you'll see that what we draw is very far from a
dropmarker.

Use non-native rendering like windows to have a proper implementation of
this if someone needs it, and remove the native code.

Remove parentfocused check which can't ever work (that attribute isn't
set anywhere).

Differential Revision: https://phabricator.services.mozilla.com/D191563
  • Loading branch information
@emilio
emilio committed Oct 24, 2023
1 parent 01b3cf4 commit 73b1942
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 110 deletions.
64 changes: 0 additions & 64 deletions widget/gtk/gtk3drawing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1184,46 +1184,6 @@ static gint moz_gtk_arrow_paint(cairo_t* cr, GdkRectangle* rect,
return MOZ_GTK_SUCCESS;
}

static gint moz_gtk_combo_box_entry_button_paint(cairo_t* cr,
GdkRectangle* rect,
GtkWidgetState* state,
gboolean input_focus,
GtkTextDirection direction) {
gint x_displacement, y_displacement;
GdkRectangle arrow_rect, real_arrow_rect;
GtkStateFlags state_flags = GetStateFlagsFromGtkWidgetState(state);
GtkStyleContext* style;

GtkWidget* comboBoxEntry = GetWidget(MOZ_GTK_COMBOBOX_ENTRY_BUTTON);
if (!comboBoxEntry) {
return MOZ_GTK_UNKNOWN_WIDGET;
}

moz_gtk_button_paint(cr, rect, state, GTK_RELIEF_NORMAL, comboBoxEntry,
direction);
calculate_button_inner_rect(comboBoxEntry, rect, &arrow_rect, direction);

if (state_flags & GTK_STATE_FLAG_ACTIVE) {
style = gtk_widget_get_style_context(comboBoxEntry);
StyleContextSetScale(style, state->image_scale);
gtk_style_context_get_style(style, "child-displacement-x", &x_displacement,
"child-displacement-y", &y_displacement, NULL);
arrow_rect.x += x_displacement;
arrow_rect.y += y_displacement;
}

GtkWidget* arrow = GetWidget(MOZ_GTK_COMBOBOX_ENTRY_ARROW);
if (!arrow) {
return MOZ_GTK_UNKNOWN_WIDGET;
}
calculate_arrow_rect(arrow, &arrow_rect, &real_arrow_rect, direction);

style = GetStyleContext(MOZ_GTK_COMBOBOX_ENTRY_ARROW, state->image_scale);
gtk_render_arrow(style, cr, ARROW_DOWN, real_arrow_rect.x, real_arrow_rect.y,
real_arrow_rect.width);
return MOZ_GTK_SUCCESS;
}

static gint moz_gtk_toolbar_paint(cairo_t* cr, GdkRectangle* rect,
GtkWidgetState* state,
GtkTextDirection direction) {
Expand Down Expand Up @@ -1766,9 +1726,6 @@ gint moz_gtk_get_widget_border(WidgetNodeType widget, gint* left, gint* top,
case MOZ_GTK_TREE_HEADER_SORTARROW:
w = GetWidget(MOZ_GTK_TREE_HEADER_SORTARROW);
break;
case MOZ_GTK_DROPDOWN_ARROW:
w = GetWidget(MOZ_GTK_COMBOBOX_ENTRY_BUTTON);
break;
case MOZ_GTK_DROPDOWN: {
/* We need to account for the arrow on the dropdown, so text
* doesn't come too close to the arrow, or in some cases spill
Expand Down Expand Up @@ -1946,24 +1903,6 @@ gint moz_gtk_get_tab_border(gint* left, gint* top, gint* right, gint* bottom,
return MOZ_GTK_SUCCESS;
}

gint moz_gtk_get_combo_box_entry_button_size(gint* width, gint* height) {
/*
* We get the requisition of the drop down button, which includes
* all padding, border and focus line widths the button uses,
* as well as the minimum arrow size and its padding
* */
GtkRequisition requisition;

gtk_widget_get_preferred_size(GetWidget(MOZ_GTK_COMBOBOX_ENTRY_BUTTON), NULL,
&requisition);
moz_gtk_sanity_preferred_size(&requisition);

*width = requisition.width;
*height = requisition.height;

return MOZ_GTK_SUCCESS;
}

gint moz_gtk_get_tab_scroll_arrow_size(gint* width, gint* height) {
gint arrow_size;

Expand Down Expand Up @@ -2317,9 +2256,6 @@ gint moz_gtk_widget_paint(WidgetNodeType widget, cairo_t* cr,
return moz_gtk_text_view_paint(cr, rect, state, direction);
case MOZ_GTK_DROPDOWN:
return moz_gtk_combo_box_paint(cr, rect, state, direction);
case MOZ_GTK_DROPDOWN_ARROW:
return moz_gtk_combo_box_entry_button_paint(cr, rect, state, flags,
direction);
case MOZ_GTK_TOOLBAR:
return moz_gtk_toolbar_paint(cr, rect, state, direction);
case MOZ_GTK_TOOLBAR_SEPARATOR:
Expand Down
11 changes: 0 additions & 11 deletions widget/gtk/gtkdrawing.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,6 @@ enum WidgetNodeType : int {
MOZ_GTK_TEXT_VIEW_TEXT_SELECTION,
/* Paints a GtkOptionMenu. */
MOZ_GTK_DROPDOWN,
/* Paints a dropdown arrow (a GtkButton containing a down GtkArrow). */
MOZ_GTK_DROPDOWN_ARROW,
/* Paints an entry in an editable option menu */
MOZ_GTK_DROPDOWN_ENTRY,

Expand Down Expand Up @@ -438,15 +436,6 @@ void moz_gtk_get_scale_metrics(GtkOrientation orient, gint* scale_width,
gint moz_gtk_get_scalethumb_metrics(GtkOrientation orient, gint* thumb_length,
gint* thumb_height);

/**
* Get the desired size of a dropdown arrow button
* width: [OUT] the desired width
* height: [OUT] the desired height
*
* returns: MOZ_GTK_SUCCESS if there was no error, an error code otherwise
*/
gint moz_gtk_get_combo_box_entry_button_size(gint* width, gint* height);

/**
* Get the desired size of a scroll arrow widget
* width: [OUT] the desired width
Expand Down
44 changes: 10 additions & 34 deletions widget/gtk/nsNativeThemeGTK.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ bool nsNativeThemeGTK::GetGtkWidgetAndState(StyleAppearance aAppearance,
aAppearance == StyleAppearance::Dualbutton ||
aAppearance == StyleAppearance::ToolbarbuttonDropdown ||
aAppearance == StyleAppearance::Menulist ||
aAppearance == StyleAppearance::MenulistButton ||
aAppearance == StyleAppearance::MozMenulistArrowButton) {
aAppearance == StyleAppearance::MenulistButton) {
aState->active &= aState->inHover;
} else if (aAppearance == StyleAppearance::Treetwisty ||
aAppearance == StyleAppearance::Treetwistyopen) {
Expand Down Expand Up @@ -224,21 +223,12 @@ bool nsNativeThemeGTK::GetGtkWidgetAndState(StyleAppearance aAppearance,
aAppearance == StyleAppearance::Dualbutton ||
aAppearance == StyleAppearance::ToolbarbuttonDropdown ||
aAppearance == StyleAppearance::Menulist ||
aAppearance == StyleAppearance::MenulistButton ||
aAppearance == StyleAppearance::MozMenulistArrowButton) {
aAppearance == StyleAppearance::MenulistButton) {
bool menuOpen = IsOpenButton(aFrame);
aState->depressed = IsCheckedButton(aFrame) || menuOpen;
// we must not highlight buttons with open drop down menus on hover.
aState->inHover = aState->inHover && !menuOpen;
}

// When the input field of the drop down button has focus, some themes
// should draw focus for the drop down button as well.
if ((aAppearance == StyleAppearance::MenulistButton ||
aAppearance == StyleAppearance::MozMenulistArrowButton) &&
aWidgetFlags) {
*aWidgetFlags = CheckBooleanAttr(aFrame, nsGkAtoms::parentfocused);
}
}

if (aAppearance == StyleAppearance::MozWindowTitlebar ||
Expand Down Expand Up @@ -362,9 +352,6 @@ bool nsNativeThemeGTK::GetGtkWidgetAndState(StyleAppearance aAppearance,
*aWidgetFlags =
IsFrameContentNodeInNamespace(aFrame, kNameSpaceID_XHTML);
break;
case StyleAppearance::MozMenulistArrowButton:
aGtkWidgetType = MOZ_GTK_DROPDOWN_ARROW;
break;
case StyleAppearance::ToolbarbuttonDropdown:
case StyleAppearance::ButtonArrowDown:
case StyleAppearance::ButtonArrowUp:
Expand Down Expand Up @@ -998,7 +985,6 @@ bool nsNativeThemeGTK::GetWidgetPadding(nsDeviceContext* aContext,
case StyleAppearance::Dualbutton:
case StyleAppearance::TabScrollArrowBack:
case StyleAppearance::TabScrollArrowForward:
case StyleAppearance::MozMenulistArrowButton:
case StyleAppearance::ToolbarbuttonDropdown:
case StyleAppearance::ButtonArrowUp:
case StyleAppearance::ButtonArrowDown:
Expand Down Expand Up @@ -1072,6 +1058,12 @@ auto nsNativeThemeGTK::IsWidgetNonNative(nsIFrame* aFrame,
return NonNative::BecauseColorMismatch;
}

bool nsNativeThemeGTK::IsWidgetAlwaysNonNative(nsIFrame* aFrame,
StyleAppearance aAppearance) {
return Theme::IsWidgetAlwaysNonNative(aFrame, aAppearance) ||
aAppearance == StyleAppearance::MozMenulistArrowButton;
}

LayoutDeviceIntSize nsNativeThemeGTK::GetMinimumWidgetSize(
nsPresContext* aPresContext, nsIFrame* aFrame,
StyleAppearance aAppearance) {
Expand Down Expand Up @@ -1101,9 +1093,6 @@ LayoutDeviceIntSize nsNativeThemeGTK::GetMinimumWidgetSize(
case StyleAppearance::TabScrollArrowForward: {
moz_gtk_get_tab_scroll_arrow_size(&result.width, &result.height);
} break;
case StyleAppearance::MozMenulistArrowButton: {
moz_gtk_get_combo_box_entry_button_size(&result.width, &result.height);
} break;
case StyleAppearance::Checkbox:
case StyleAppearance::Radio: {
const ToggleGTKMetrics* metrics = GetToggleMetrics(
Expand Down Expand Up @@ -1273,8 +1262,7 @@ nsNativeThemeGTK::WidgetStateChanged(nsIFrame* aFrame,
aAttribute == nsGkAtoms::visuallyselected ||
aAttribute == nsGkAtoms::focused || aAttribute == nsGkAtoms::readonly ||
aAttribute == nsGkAtoms::_default ||
aAttribute == nsGkAtoms::menuactive || aAttribute == nsGkAtoms::open ||
aAttribute == nsGkAtoms::parentfocused) {
aAttribute == nsGkAtoms::menuactive || aAttribute == nsGkAtoms::open) {
*aShouldRepaint = true;
return NS_OK;
}
Expand Down Expand Up @@ -1360,17 +1348,6 @@ nsNativeThemeGTK::ThemeSupportsWidget(nsPresContext* aPresContext,
case StyleAppearance::MozWindowTitlebarMaximized:
case StyleAppearance::MozWindowDecorations:
return !IsWidgetStyled(aPresContext, aFrame, aAppearance);

case StyleAppearance::MozMenulistArrowButton:
if (aFrame && aFrame->GetWritingMode().IsVertical()) {
return false;
}
// "Native" dropdown buttons cause padding and margin problems, but only
// in HTML so allow them in XUL.
return (!aFrame ||
IsFrameContentNodeInNamespace(aFrame, kNameSpaceID_XUL)) &&
!IsWidgetStyled(aPresContext, aFrame, aAppearance);

default:
break;
}
Expand All @@ -1381,8 +1358,7 @@ nsNativeThemeGTK::ThemeSupportsWidget(nsPresContext* aPresContext,
NS_IMETHODIMP_(bool)
nsNativeThemeGTK::WidgetIsContainer(StyleAppearance aAppearance) {
// XXXdwh At some point flesh all of this out.
if (aAppearance == StyleAppearance::MozMenulistArrowButton ||
aAppearance == StyleAppearance::Radio ||
if (aAppearance == StyleAppearance::Radio ||
aAppearance == StyleAppearance::RangeThumb ||
aAppearance == StyleAppearance::Checkbox ||
aAppearance == StyleAppearance::TabScrollArrowBack ||
Expand Down
1 change: 1 addition & 0 deletions widget/gtk/nsNativeThemeGTK.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class nsNativeThemeGTK final : public mozilla::widget::Theme {
// subtle sizing changes. The non-native theme can basically draw at any size,
// so we prefer to have consistent sizing information.
enum class NonNative { No, Always, BecauseColorMismatch };
static bool IsWidgetAlwaysNonNative(nsIFrame*, StyleAppearance);
NonNative IsWidgetNonNative(nsIFrame*, StyleAppearance);

mozilla::LayoutDeviceIntSize GetMinimumWidgetSize(
Expand Down
1 change: 0 additions & 1 deletion xpcom/ds/StaticAtoms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,6 @@
Atom("param", "param"),
Atom("parameter", "parameter"),
Atom("parent", "parent"),
Atom("parentfocused", "parentfocused"),
Atom("parsererror", "parsererror"),
Atom("part", "part"),
Atom("password", "password"),
Expand Down

0 comments on commit 73b1942

Please sign in to comment.