1216582 - [gtk3] Scrollbar buttons not drawn correctly

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Jan Horak [:jhorak]]

Attachment: scrollbars buttons demo.png

Scrollbar buttons (steppers) are wrongly paint in some GTK3 themes. See screenshot for details.

Comment 1

[Jan Horak [:jhorak]]

Attachment: scrollbar-buttons-fix.patch

Fix is basically based on https://github.com/GNOME/gtk/blob/74e02842bbf832c32fb6d36692b760aefb05682e/gtk/gtkrange.c#L1772

Comment 2

[Karl Tomlinson (:karlt)]

Comment on attachment 8676294 [details] [diff] [review]
scrollbar-buttons-fix.patch

Thanks!

>     gtk_style_context_add_class(style, GTK_STYLE_CLASS_SCROLLBAR);  

Can you remove this line, please, as the GtkScrollbar adds that for us?

>+    if (arrow_angle == ARROW_RIGHT) {
>+        gtk_style_context_add_class (style, GTK_STYLE_CLASS_RIGHT);
>+    } else if (arrow_angle ==  ARROW_DOWN) {
>+        gtk_style_context_add_class (style, GTK_STYLE_CLASS_BOTTOM);
>+    } else if (arrow_angle ==  ARROW_LEFT) {
>+        gtk_style_context_add_class (style, GTK_STYLE_CLASS_LEFT);
>+    } else {
>+        gtk_style_context_add_class (style, GTK_STYLE_CLASS_TOP);

Please remove extra space before ARROW_DOWN and ARROW_LEFT.

Please remove spaces between gtk_style_context_add_class and '('.

>+    gdouble arrow_size = MIN (rect->width, rect->height) * arrow_scaling;

Please remove space in 'MIN ('.

Comment 3

[Jan Horak [:jhorak]]

Attachment: scrollbar-buttons-fix.patch

Thanks for the review, fixing mentioned issues and copying previous review+.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98a89494be5e

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad80366135f3

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ad80366135f3

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 8677289 [details] [diff] [review]
scrollbar-buttons-fix.patch

Approval Request Comment
[Feature/regressing bug #]: GTK3 (bug 1186229)
[User impact if declined]:
Scrollbar buttons half missing on themes that have buttons.
[Describe test coverage new/current, TreeHerder]:
No automated tests.  Been on 44 for some time.
[Risks and why]:
Risks are confined to scrollbar buttons.
The reward out-weighs the risk here.
[String/UUID change made/needed]: none.

Comment 7

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Marking as affected for 43.

Comment 8

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8677289 [details] [diff] [review]
scrollbar-buttons-fix.patch

Fix for GKT3, aiming to land this for beta 4 on Monday.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/807e612c17ef