After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778413 - Dark theme does not work
Dark theme does not work
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: preferences
Flatpak Stable Channel
Other Linux
: Normal minor
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-09 21:27 UTC by David Berg
Modified: 2017-03-01 01:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot of the bug (36.76 KB, image/png)
2017-02-09 21:27 UTC, David Berg
  Details
video showing dark theme does not work when host theme isn't Adwaita (915.60 KB, video/mp4)
2017-02-10 00:10 UTC, David Berg
  Details
scrollbar warnings, resize cursor mismatch (1.28 MB, video/mp4)
2017-03-01 00:06 UTC, David Berg
  Details
resize cursor differences (836.07 KB, video/mp4)
2017-03-01 00:48 UTC, David Berg
  Details
pnl: use gdk_cursor_new_from_name() (1.97 KB, patch)
2017-03-01 01:44 UTC, Christian Hergert
committed Details | Review

Description David Berg 2017-02-09 21:27:18 UTC
Created attachment 345381 [details]
screenshot of the bug

Builder's dark theme cannot be enabled. Attached is a screenshot of the "Dark Theme" switch enabled, but the application is still using the default light theme. I tried running from the command line with the -v switch, but nothing useful turned up.

Running Builder 3.22.4 on Flatpak version 0.8.2 in elementary 0.4 Loki (based on Ubuntu 16.04 LTS).
Comment 1 Christian Hergert 2017-02-09 22:03:04 UTC
Interesting, it seems to work here so there must be something more tricky going on.
Comment 2 Christian Hergert 2017-02-09 22:15:52 UTC
I wonder if this is caused by the host theme not being available in the flatpak runtime (and therefore we fallback to Adwaita). But then since we can't locate that theme even applying the dark mode doesn't work.

We might want to force Adwaita in flatpak (without using the fallback code) since that is what is available anyway (and honestly I want to force Adwaita always anyway since we have so much custom CSS for it).
Comment 3 David Berg 2017-02-10 00:08:48 UTC
You might be right. I tried changing the host theme to Adwaita and the dark theme works. See video.
Comment 4 David Berg 2017-02-10 00:10:07 UTC
Created attachment 345389 [details]
video showing dark theme does not work when host theme isn't Adwaita
Comment 5 Christian Hergert 2017-02-28 16:10:23 UTC
I think this should be fixed on our Nightly Channel, and I'd really appreciate if you could update and let me know.

In particular, I think there is a lower-level bug here in gtk+ that has some special heuristics about switching between normal/dark-variants. I suspect it's keying off the theme name coming from the system via GSettings (which would match your host system since we access gsettings/dconf from the host).

However, we don't have that theme in the flatpak (it would look like crap anyway because all of our custom CSS which is not implemented for that theme).

So what I've done in the meantime, is just force Adwaita for Builder as soon as possible during startup if we're inside of Flatpak. Adwaita is really the best experience we can give right now unless we 1) have someone to port custom CSS to the other "major" themes or 2) find a way to abstract the CSS in a more generic way. Lapo (one of the Adwaita authors) believes we can do 2, but that requires someone of his skill-set to have free time (ie: not me).

Anywho, long-winded version of "update and report back to me!" :)
Comment 6 David Berg 2017-03-01 00:05:25 UTC
Just tested the nightly channel (v3.23.90+168-g63a88b3) and the dark theme works as expected. Thanks!

I definitely noticed some style changes between this and the stable channel, especially the system monitor/todo/terminal bar--it's much thicker and the active tab is highlighted now. I'm guessing even though Adwaita was being used as a fallback in the stable channel, the custom CSS for Builder was not being applied.

This is probably worthy of a separate bug report, but I'm attaching a video that shows Builder spitting out a bunch of repeated scrollbar warnings when resizing the terminal window:

(gnome-builder:2): Gtk-WARNING **: Allocating size to GtkScrollbar 0x3dcfce0 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?

I don't ever remember Builder using a scrollbar for the terminal.

You might also notice in the video that the resize cursor is out-of-theme. Perhaps the cursor themes are also different between the flatpak and host system?

Personally, I don't mind using the Adwaita theme for Builder. It looks great! Some of the 3rd-party GTK+ themes might be open to option 1), but Daniel from elementary is definitely going in the opposite direction[1], which makes sense for elementary OS (they only guarantee consistency for their own stuff). 

[1]: https://github.com/elementary/stylesheet/projects
Comment 7 David Berg 2017-03-01 00:06:17 UTC
Created attachment 346943 [details]
scrollbar warnings, resize cursor mismatch
Comment 8 Christian Hergert 2017-03-01 00:17:04 UTC
(In reply to David Berg from comment #6)
> (gnome-builder:2): Gtk-WARNING **: Allocating size to GtkScrollbar 0x3dcfce0
> without calling gtk_widget_get_preferred_width/height(). How does the code
> know the size to allocate?

I think this is a Gtk bug. It should be okay though.

> I don't ever remember Builder using a scrollbar for the terminal.

Back around 3.20, we tried not using a scrollbar and relying on overlay scrollbars. But that isn't really safe to do, so we changed to doing things more like gnome-terminal handles them.

> You might also notice in the video that the resize cursor is out-of-theme.
> Perhaps the cursor themes are also different between the flatpak and host
> system?

Are you on HiDPI by chance? Does elementary use an alternate cursor theme than Adwaita (maybe it's missing something, although it'd be strange if it were since that is such a common cursor).

> Personally, I don't mind using the Adwaita theme for Builder. It looks
> great! Some of the 3rd-party GTK+ themes might be open to option 1), but
> Daniel from elementary is definitely going in the opposite direction[1],
> which makes sense for elementary OS (they only guarantee consistency for
> their own stuff). 

I think Daniel is making the right decision here. The theme overrides belong upstream so that when we change things in Builder, we can fix it in their theme too.
Comment 9 David Berg 2017-03-01 00:25:45 UTC
(In reply to Christian Hergert from comment #8)
> Are you on HiDPI by chance? Does elementary use an alternate cursor theme
> than Adwaita (maybe it's missing something, although it'd be strange if it
> were since that is such a common cursor).

Not on HiDPI. It would appear that elementary OS indeed uses its own cursor theme, "elementary." Changing it to "Adwaita" fixes the issue.
Comment 10 Christian Hergert 2017-03-01 00:28:44 UTC
I don't think we're doing anything particularly weird, but the code in question is here:

https://git.gnome.org/browse/gnome-builder/tree/contrib/pnl/pnl-dock-bin.c#n907
Comment 11 David Berg 2017-03-01 00:47:52 UTC
All I can say is that the resize cursors appear to be different between a flatpak and a native elementary app. I'd have to take a look at the complete sets of each to know which cursors are in which theme. I'm attaching another video to show the differences.

Let me know if there's a better place to continue this tangent.
Comment 12 David Berg 2017-03-01 00:48:22 UTC
Created attachment 346944 [details]
resize cursor differences
Comment 13 Christian Hergert 2017-03-01 01:03:36 UTC
(In reply to David Berg from comment #11)
> All I can say is that the resize cursors appear to be different between a
> flatpak and a native elementary app. I'd have to take a look at the complete
> sets of each to know which cursors are in which theme. I'm attaching another
> video to show the differences.

This might just be an issue on Xorg (elementary does not support wayland yet, right?)

Either way, hopefully commit 5119aa3f6041700e9c3b9df3a2ea850c91c48102 will be a work around for something attractive until I understand the problem better.

I'm not sure how the plumbing works for communicating with cursors outside the runtime, and I'll have to go read a bunch of code to answer that.
Comment 14 Christian Hergert 2017-03-01 01:07:37 UTC
It looks like gtk has a test in-tree you can use to verify if the elementary cursor theme is missing icons by name (although you have to build Gtk from source).

It's in testsuite/gtk/check-cursor-names and it should output something like:

/check-cursor-names/nw-resize: OK
/check-cursor-names/n-resize: OK
/check-cursor-names/ne-resize: OK
/check-cursor-names/w-resize: OK
/check-cursor-names/e-resize: OK
/check-cursor-names/sw-resize: OK
/check-cursor-names/s-resize: OK
/check-cursor-names/se-resize: OK
/check-cursor-names/col-resize: OK
/check-cursor-names/row-resize: OK
/check-cursor-names/dnd-ask: OK
/check-cursor-names/copy: OK
/check-cursor-names/move: OK
/check-cursor-names/alias: OK
/check-cursor-names/no-drop: OK
/check-cursor-names/none: OK
/check-cursor-names/pointer: OK
/check-cursor-names/text: OK
/check-cursor-names/crosshair: OK
/check-cursor-names/progress: OK
Comment 15 Christian Hergert 2017-03-01 01:09:15 UTC
If it turns out that elementary just has a missing cursor by name, then I'd like to revert commit 5119aa3f since changing cursor themes inside a window can be a bit jarring.
Comment 16 David Berg 2017-03-01 01:21:44 UTC
You are correct, elementary does not support wayland.

Building GTK+ from source is a little much for me, but I think this is the source for the elementary cursor theme[1]. I took a look through the cursors folder[2] and I don't see any of the "-resize" names you posted above.

[1]: https://github.com/snwh/elementary-cursors
[2]: https://github.com/snwh/elementary-cursors/tree/master/elementary/cursors
Comment 17 Christian Hergert 2017-03-01 01:37:20 UTC
I see this when testing with elementary cursor theme:

Gdk-Message: Unable to load sb_v_double_arrow from the cursor theme
Gdk-Message: Unable to load sb_h_double_arrow from the cursor theme

So I'm inclined to say that it is an elementary cursor issue.
Comment 18 Christian Hergert 2017-03-01 01:40:21 UTC
So gtk has changed how they do cursor names since our panel implementation was made. So we can probably track that and improve things a bit.

Snip from gtk:

gdk_cursor_new_from_name (gtk_widget_get_display (widget),
          priv->orientation == GTK_ORIENTATION_HORIZONTAL
                                              ? "col-resize" : "row-resize");
Comment 19 Christian Hergert 2017-03-01 01:44:16 UTC
Created attachment 346945 [details] [review]
pnl: use gdk_cursor_new_from_name()

We should be consistently using cursor names everywhere.
Comment 20 Christian Hergert 2017-03-01 01:47:05 UTC
Comment on attachment 346945 [details] [review]
pnl: use gdk_cursor_new_from_name()

Attachment 346945 [details] pushed as 421bbd4 - pnl: use gdk_cursor_new_from_name()
Comment 21 David Berg 2017-03-01 01:47:51 UTC
I think the flatpak is already using the Adwaita cursor theme regardless of the host system cursor theme, because in the video I uploaded the resize cursors around the edges on Builder are definitely from Adwaita, even when the host is set to "elementary." It's just that row-resize cursor on the middle panel that doesn't render properly when the host is set different from the flatpak.