Closed Bug 1454897 Opened 6 years ago Closed 6 years ago

Honour Ambiance scrollbar theme

Categories

(Core :: Widget: Gtk, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

Details

Attachments

(1 file)

Bug 1355143 fixes scrollbar track size. We should also honor Ambiance scrollbar theme and resize scrollbar thumb when mouse is/isn't over it.
Assignee: nobody → stransky
Summary: Honor Ambiance scrollbar theme → Honour Ambiance scrollbar theme
Comment on attachment 8968860 [details]
Bug 1454897 - Ubuntu/Ambiance - Render scrollbar thumb with different sizes in active/normal state,

https://reviewboard.mozilla.org/r/237584/#review243376

::: widget/gtk/gtk3drawing.cpp:1026
(Diff revision 1)
> -    gtk_render_slider(style, cr,
> -                      rect.x,
> -                      rect.y,
> -                      rect.width,
> -                      rect.height,
> -                     (widget == MOZ_GTK_SCROLLBAR_THUMB_HORIZONTAL) ?
> +    GtkOrientation orientation = (widget == MOZ_GTK_SCROLLBAR_THUMB_HORIZONTAL) ?
> +        GTK_ORIENTATION_HORIZONTAL : GTK_ORIENTATION_VERTICAL;
> +
> +    if (!(state_flags & (GTK_STATE_FLAG_ACTIVE|GTK_STATE_FLAG_PRELIGHT))) {
> +        const ScrollbarGTKMetrics* metrics = GetScrollbarMetrics(orientation);
> +        const ScrollbarGTKMetrics* metricsSelected =

The 'selected' is probably not correct word there, because you can't select thumb element. Use 'metricsActive' or 'metricsPrelight'

::: widget/gtk/gtk3drawing.cpp:1029
(Diff revision 1)
> +        if (metrics->size.thumb < metricsSelected->size.thumb) {
> +            GtkBorder marginDiff =
> +                metrics->margin.scrollbar - metricsSelected->margin.scrollbar;
> +            Inset(&rect, marginDiff);

Instead of applying InsetByMargin on line 1019 and calculating difference between metrics and matricsSelected and doing Inset again, couldn't you just get margin from the correct metrics (normal/active) and apply it?

This also remove need for the operator- which is not very common in mozilla source base.

::: widget/gtk/gtk3drawing.cpp:2992
(Diff revision 1)
>      }
>  
>      // GTK version > 3.20
>      // scrollbar
>      metrics->border.scrollbar = GetMarginBorderPadding(style);
> +    metrics->margin.scrollbar = GetMargin(style);

Just use gtk_style_context_get_margin, there's no need to wrap it in GetMargin().
Comment on attachment 8968860 [details]
Bug 1454897 - Ubuntu/Ambiance - Render scrollbar thumb with different sizes in active/normal state,

https://reviewboard.mozilla.org/r/237584/#review243376

> Instead of applying InsetByMargin on line 1019 and calculating difference between metrics and matricsSelected and doing Inset again, couldn't you just get margin from the correct metrics (normal/active) and apply it?
> 
> This also remove need for the operator- which is not very common in mozilla source base.

I can't do that as the first margin is for "Thumb" and the secont is margin difference between active/inactive scrollbar. But I can polish that somehow.
Comment on attachment 8968860 [details]
Bug 1454897 - Ubuntu/Ambiance - Render scrollbar thumb with different sizes in active/normal state,

https://reviewboard.mozilla.org/r/237584/#review244570

Please also rerun the try.

::: widget/gtk/gtk3drawing.cpp:44
(Diff revision 4)
>  #endif
>  
> +static GtkBorder
> +operator-(const GtkBorder& first, const GtkBorder& second)
> +{
> +    GtkBorder tmp;

better name result than tmp.

::: widget/gtk/gtk3drawing.cpp:3086
(Diff revision 4)
> +          metrics->margin.thumb += metrics->margin.scrollbar -
> +                                   metricsActive->margin.scrollbar;

Can't you use border (ie. margin+border+padding) instead of just margin there?
(metricsActive->border.scrollbar + metricsActive->border.track) -  (metrics->border.scrollbar + metrics->border.track)

In case the theme changes padding instead of margin.
Attachment #8968860 - Flags: review?(jhorak) → review+
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/05f610551597
Ubuntu/Ambiance - Render scrollbar thumb with different sizes in active/normal state, r=jhorak
https://hg.mozilla.org/mozilla-central/rev/05f610551597
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.