[gtk3] Scrollbar buttons not drawn correctly

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jhorak, Assigned: jhorak)

Tracking

40 Branch
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed, firefox44 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Scrollbar buttons (steppers) are wrongly paint in some GTK3 themes. See screenshot for details.
Posted patch scrollbar-buttons-fix.patch (obsolete) — Splinter Review
Fix is basically based on https://github.com/GNOME/gtk/blob/74e02842bbf832c32fb6d36692b760aefb05682e/gtk/gtkrange.c#L1772
Assignee: nobody → jhorak
Attachment #8676294 - Flags: review?(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 ('.
Attachment #8676294 - Flags: review?(karlt) → review+
Thanks for the review, fixing mentioned issues and copying previous review+.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98a89494be5e
Attachment #8676294 - Attachment is obsolete: true
Attachment #8677289 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ad80366135f3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
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.
Attachment #8677289 - Flags: approval-mozilla-beta?
Marking as affected for 43.
Comment on attachment 8677289 [details] [diff] [review]
scrollbar-buttons-fix.patch

Fix for GKT3, aiming to land this for beta 4 on Monday.
Attachment #8677289 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.