David Baron :dbaron: Reporter
	Description • 9 years ago

Since GTK3 was switched on by default, selects on bugzilla (which I believe are default-styled) have been much shorter in height than GTK2 selects, and the text has been a bit off-center.  (This is somewhat similar to bug 1187385 for text inputs.)

I'm running Ubuntu 15.04; I don't think I've changed theme settings much, if at all, although I've upgraded from earlier Ubuntu versions.

	David Baron :dbaron: Reporter
	Comment 2 • 9 years ago

(from gnome-terminal comparison)

	David Baron :dbaron: Reporter
	Comment 3 • 9 years ago

...er, gnome-terminal preferences

	Lee Salzman [:lsalzman]
	Comment 4 • 9 years ago

This occurs for me on a 43 nightly, I can confirm.

	Andrew Comminos [:acomminos] Assignee
	Comment 5 • 9 years ago

Looks like this is caused by an omission of the GTK theme's style padding for dropdown boxes in HTML;

https://dxr.mozilla.org/mozilla-central/rev/c0abc2a6e11f52761366e029eb1bae4c9864a8a3/widget/gtk/gtk3drawing.c#2765

We should probably use it everywhere for consistency and usability's sake.

	Andrew Comminos [:acomminos] Assignee
	Comment 6 • 9 years ago

Here's a simple patch to use the style padding in HTML.

This may break some tests that expect the combo box to fit within certain bounds, so depending on this try push there may be followup patches;

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2ac87bcb689

Thanks!

	Karl Tomlinson (:karlt)
	Comment 7 • 9 years ago

Comment on attachment 8660193 [details] [diff] [review]
Use GTK style padding for dropdown boxes in HTML.

This is part of the solution, but the arrow is not positioned correctly.

The drawing code should be consistent, which means calling calculate_button_inner_rect from moz_gtk_combo_box_paint with ignore_focus false, which makes that parameter and the inhtml parameter to moz_gtk_combo_box_paint unnecessary.  I guess we should keep the aWidgetFlags code for NS_THEME_DROPDOWN in nsNativeThemeGTK::GetGtkWidgetAndState() for GTK2 because it means something a little different there.

	Karl Tomlinson (:karlt)
	Comment 8 • 9 years ago

See also https://bugzilla.mozilla.org/show_bug.cgi?id=1073117#c48

	Andrew Comminos [:acomminos] Assignee
	Comment 9 • 9 years ago

Thanks, here's an updated patch with correct inner rect calculation and flag removal.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75d9e54c2290

	Andrew Comminos [:acomminos] Assignee
	Comment 10 • 9 years ago

Regarding the vertical off-center issue; the widget code appears to be providing the same (correct) padding for the top and bottom of the <select> tag, so the issue is likely elsewhere. I suspect that it may have something to do with the metrics of the font used (Fira Sans); changing fonts makes the text appear much better centered.

	Martin Stránský [:stransky] (ni? me)
	Comment 11 • 9 years ago

Comment on attachment 8662691 [details] [diff] [review]
Use GTK style padding for dropdown boxes in HTML. Carry r=karlt

This patch makes bugzilla controls extra large on default Gtk3 theme on Fedora. The font approach looks more promising. There's also a bug in gtk - it returns large border/padding values but renders with smaller numbers here:

https://bugzilla.redhat.com/show_bug.cgi?id=1205651#c3

	Karl Tomlinson (:karlt)
	Comment 12 • 9 years ago

(In reply to Martin Stránský from comment #11)
> Comment on attachment 8662691 [details] [diff] [review]
> This patch makes bugzilla controls extra large on default Gtk3 theme on
> Fedora.

It does make the controls larger, but it seems consistent with native widgets.
(It is still one pixel short, but I don't know what is causing that difference.)

Are you happy with this patch now?

	Martin Stránský [:stransky] (ni? me)
	Comment 13 • 9 years ago

Yes, the patch if fine (although it makes the comboboxes 1 pix. lower than original gtk3 ones). 

IMHO we should match all toolkit elements to the same size, for instance buttons and editable dropdowns are still undersized.

	Martin Stránský [:stransky] (ni? me)
	Comment 14 • 9 years ago

Unfortunately bugzilla is not very good platform for the form testing. Some elements (for instance the summary entry box) are shrink by the html page by some pixels (for instance removed bottom padding) which greatly confused me.

I suggest to test/compare all form sizes on single test page, like this one:
https://www.lehigh.edu/~inwww/form-test.html

and compare those sizes with reference gtk3 application which may be gtk3-demo or gtk3-widget-factory.

For what I see now (with Bug 1187385 fixed) on Fedora/Adwaita with this patch:

Firefox GtkEntry is 27pix high - gtk3 one is 26pix high
Firefox entry is 25pix high - gtk3 one is 26pix high

I'll try to align the sizes with gtk3 and also align the button sizes. I see many elements are modified by forms.css (like checkboxes and so) which may be worth to investigate.

	Martin Stránský [:stransky] (ni? me)
	Comment 15 • 9 years ago

It appears that Gkt draws a shadow under the widget which we don't draw and the widget looks bigger then.

	Pulsebot
	Comment 16 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/938041e183e5

	Pulsebot
	Comment 17 • 9 years ago

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca07f8f6a2bd

	Martin Stránský [:stransky] (ni? me)
	Comment 18 • 9 years ago

(In reply to Karl Tomlinson (ni?:karlt) from comment #12)
> It does make the controls larger, but it seems consistent with native
> widgets.
> (It is still one pixel short, but I don't know what is causing that
> difference.)

That's caused by external leading font property. The external leading adds pixels between text rows but Firefox also uses that for single-line elements. When browser.display.normal_lineheight_calc_control is set to 0 the elements match gtk3. 

The external leading is loaded from each font which also explains why different fonts give different results.

	Pulsebot
	Comment 19 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/390cb35d9245

	Carsten Book [:Tomcat]
	Comment 20 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/390cb35d9245