GNOME Bugzilla – Bug 778905
Frame: documented flat style class is not usable
Last modified: 2017-03-05 18:30:08 UTC
From the docs: CSS nodes frame ├── border ├── <label widget> ╰── <child> GtkFrame has a main CSS node with name frame and a subnode with name border. The border node is used to render the visible border. The style class .flat can appear with the main node. Adding .flat to the main node, which is the only thing we can do in widget code, doesn't work. In fact, the theme selects for .flat on the frame > border node: frame > border, .frame { box-shadow: none; margin: 0; padding: 0; border-radius: 0; border: 1px solid #1b1f20; } frame > border.flat, .frame.flat { border-style: none; } frame > border:backdrop, .frame:backdrop { border-color: #202425; } This means that we cannot (without impractical acrobatics) use the .flat class from code in 3.22, which is inconvenient and inconsistent with the documentation.
Created attachment 346181 [details] [review] themes: Actually apply the frame.flat style class The docs say that this class should be put on the frame node, and that’s all we can do from C code, but the CSS was selecting on the border node. The result was that adding .flat did not disable the border as expected.
Attachment 346181 [details] pushed as 0c20604 - themes: Actually apply the frame.flat style class
I think some things have a border now when they didn't before in gtk+ 3.22.8 https://i.imgur.com/Qp3h47p.png
Many thanks for the report. cc_shell_category_view creates a GtkFrame and applies shadow_type GTK_SHADOW_NONE. In gtkframe.c, that shadow type is applied by adding .flat to the border_gadget, not the main node. D'oh! The fix will be either * to apply that class to the main gadget instead, or * to restore the old frame > border.flat selector, so that either combination works I think the first of these is more correct, given what the documentation says about adding this class manually.
Fixed by https://bugzilla.gnome.org/show_bug.cgi?id=779005 ?
Indeed, thanks for that!
(In reply to Daniel Boles from comment #4) > Many thanks for the report. > > cc_shell_category_view creates a GtkFrame and applies shadow_type > GTK_SHADOW_NONE. > > In gtkframe.c, that shadow type is applied by adding .flat to the > border_gadget, not the main node. D'oh! > > The fix will be either > * to apply that class to the main gadget instead, or > * to restore the old frame > border.flat selector, so that either > combination works > > I think the first of these is more correct, given what the documentation > says about adding this class manually. Hm, it seems though as there are 3rd party themes which use border.flat. https://codesearch.debian.net/search?q=border.flat Wasn't the idea of 3.22 to avoid such breakages?
Well, now the code does what it should have been doing all along, according to the documentation. It would've been nicer if theme authors had reported the inconsistency, rather than just rolling with it. :P In terms of relevant things on that page, I see * the old versions of GTK+ official themes * a MATE theme that also selects on frame>border and therefore works anyway * Greybird and Arc, which do only select on frame > border.flat My completely unauthoritative preference would be to file bugs against the latter 2 themes to get them updated for the corrected location of the .flat class However, if the maintainers wanted to maintain absolute backwards compatibility with those themes using .flat in the old, wrong place - I suppose the code could be updated to put .flat on _both_ nodes. Thoughts?
Or, of course, revert the whole lot, and just change the documentation to say frame>border.flat instead. I had discounted that, saying there was no way to add .flat to the internal border node from code, but as quickly became clear (and I have since clarified in the docs), that's precisely what set_shadow_type(NONE) does - so this could all have been a misguided waste of time... If so, I apologise for rushing out the change and any hassle I caused.
(In reply to Daniel Boles from comment #9) > Or, of course, revert the whole lot, and just change the documentation to > say frame>border.flat instead. As confirmed by Matthias, this is the correct option, and I've just pushed it. To reiterate: > I apologise for rushing out the change and any hassle I caused.