1408335 - [CSD] Wrong titlebar button icons on some themes

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[Martin Stránský [:stransky] (ni? me)]

Follow up from Bug 1399611 comment 15:

Hi all,
I am testing the CSD patch on Debian via flatpak. It works quite well except for the fact that superimposed to my icon theme buttons (close and maximize, minimize if present)there is another set of gray icons. Changing the theme, my icons change but the superimposed ones persist. I can't figure out if it is a Flatpak issue or it's due to the program itself.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

I noticed that that LWT themes in Windows (I tested W10) also provide icons - we should also implement that for Linux.

Comment 2

[Michael Heyns]

Not sure if this warrants a separate ticket:

Just tried CSD in latest nightly and so far it's great! I did notice the close button is slightly off when using the popular Arc Gtk theme https://github.com/horst3180/arc-theme

Close button is not centred and fill is the wrong colour.

See https://imgur.com/a/raeCR for screenshot comparison of densities

Comment 3

[N. W.]

Running KDE neon as the distro here, which basically is the official KDE distro and is always up-to-date with the latest KDE software: https://neon.kde.org/

KDE neon is currently based on Ubuntu 16.04, which uses GTK+ 3.18. This is what it looks like here:

https://bugzilla.mozilla.org/attachment.cgi?id=8929825

As you can see, the window controls in the tab bar look odd, which probably is: https://bugzilla.mozilla.org/show_bug.cgi?id=1408335

The other issues like the duplicated window controls and so on are being tracked in: https://bugzilla.mozilla.org/show_bug.cgi?id=1417933

Comment 4

[3ship]

(In reply to Michael Heyns from comment #2)
> Not sure if this warrants a separate ticket:
> 
> Just tried CSD in latest nightly and so far it's great! I did notice the
> close button is slightly off when using the popular Arc Gtk theme
> https://github.com/horst3180/arc-theme
> 
> Close button is not centred and fill is the wrong colour.
> 
> See https://imgur.com/a/raeCR for screenshot comparison of densities

I can confirm this behaviour. To expand on it:

Here's what it looks like with minimize and maximize buttons: https://imgur.com/a/nlng4
There appears to be a second set of buttons on top of the regular Arc Theme buttons, like described in comment #0.

It appears, this issue is present throughout all GTK+ themes I have installed, including Adwaita(-dark), Adapta, Numix-frost and Paper.

Using Nightly (20171120)

Comment 5

[Sean L]

I'm also seeing that behavior. See attached images.

https://imgur.com/a/lEWvE

This is 59.0a1 (2017-11-22) (64-bit). Thanks a bunch!

Comment 6

[Martin Stránský [:stransky] (ni? me)]

It's caused by Gtk+ themes with a specific button.titlebutton theming.

For instance Arc-Dark defines:

.titlebar button.titlebutton.close,
.titlebar button.titlebutton.maximize,
.titlebar button.titlebutton.minimize {
    color: transparent;
    ....
}

.titlebar button.titlebutton.close {
 background-image: -gtk-scaled(url("assets/titlebutton-close-dark.png"), url("assets/[e-mail]")); }

which means default Gtk+ titlebar button is transparent (invisible) and it's completely replaced by background-image.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1408335 - Render titlebar icons as themed ones,

Render titlebar button icons as a part of -moz-window-button-* appearence. It allows us to
theme the icons accordingly. We add a GtkImage widget to header bar buttons as Gtk+ does and
store icon pixel data there and render it at moz_gtk_header_bar_button_paint() as a part
of the buttons.

The patch also implements a cache for moz_gtk_titlebar_button_get_metrics() to get
titlebar button metrics faster.

Review commit: https://reviewboard.mozilla.org/r/211298/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/211298/

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8941009 [details]
Bug 1408335 - Render titlebar icons as themed ones,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211298/diff/1-2/

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8941009 [details]
Bug 1408335 - Render titlebar icons as themed ones,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211298/diff/2-3/

Comment 11

[Jan Horak [:jhorak]]

Comment on attachment 8941009 [details]
Bug 1408335 - Render titlebar icons as themed ones,

https://reviewboard.mozilla.org/r/211298/#review222310

::: widget/gtk/gtkdrawing.h:313
(Diff revision 3)
>    MOZ_GTK_SCROLLED_WINDOW,
>    /* Paints a GtkHeaderBar */
>    MOZ_GTK_HEADER_BAR,
>    /* Paints a GtkHeaderBar in maximized state */
>    MOZ_GTK_HEADER_BAR_MAXIMIZED,
>    /* Paints GtkHeaderBar title buttons */

Please add comment that you expect TOOLBAR_BUTTONS number of buttons starting with MOZ_GTK_HEADER_BAR_BUTTON_CLOSE (because you use this value as starting point for pointer to array of buttons cache).

Comment 12

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8941009 [details]
Bug 1408335 - Render titlebar icons as themed ones,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211298/diff/3-4/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fbf1c39eb3b
Render titlebar icons as themed ones, r=jhorak

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8fbf1c39eb3b

Comment 16

[3ship]

There appears to be a regression of this bug, in build 20180303100406
Window buttons look like they did, before bug 1408335 was fixed.

Comment 17

[Bruno Pagani]

I can confirm that there is a regression that appeared y-day or the day before.

Comment 18

[Martin Stránský [:stransky] (ni? me)]

(In reply to Bruno Pagani from comment #17)
> I can confirm that there is a regression that appeared y-day or the day
> before.

Can you please file another bug for that, attach a screenshot and specify which theme do you run. Thanks!

Comment 19

[Bruno Pagani]

(In reply to Martin Stránský [:stransky] from comment #18)
> (In reply to Bruno Pagani from comment #17)
> > I can confirm that there is a regression that appeared y-day or the day
> > before.
> 
> Can you please file another bug for that, attach a screenshot and specify
> which theme do you run. Thanks!

Done: https://bugzilla.mozilla.org/show_bug.cgi?id=1443481

Comment 22

[smonff]

I have no idea why this bug just got fixed for me with Firefox 84.0.1 and Ubuntu using Plasma. This is really a major improvement.