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 778905 - Frame: documented flat style class is not usable
Frame: documented flat style class is not usable
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2017-02-19 12:18 UTC by Daniel Boles
Modified: 2017-03-05 18:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
themes: Actually apply the frame.flat style class (2.60 KB, patch)
2017-02-19 12:27 UTC, Daniel Boles
committed Details | Review

Description Daniel Boles 2017-02-19 12:18:50 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.
Comment 1 Daniel Boles 2017-02-19 12:27:53 UTC
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.
Comment 2 Daniel Boles 2017-02-19 12:34:20 UTC
Attachment 346181 [details] pushed as 0c20604 - themes: Actually apply the frame.flat style class
Comment 3 Hussam Al-Tayeb 2017-02-21 11:16:07 UTC
I think some things have a border now when they didn't before in gtk+ 3.22.8
https://i.imgur.com/Qp3h47p.png
Comment 4 Daniel Boles 2017-02-21 11:45:58 UTC
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.
Comment 5 Timm Bäder 2017-02-21 11:48:37 UTC
Fixed by https://bugzilla.gnome.org/show_bug.cgi?id=779005 ?
Comment 6 Daniel Boles 2017-02-21 11:51:47 UTC
Indeed, thanks for that!
Comment 7 Michael Biebl 2017-03-03 19:47:16 UTC
(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?
Comment 8 Daniel Boles 2017-03-03 19:54:56 UTC
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?
Comment 9 Daniel Boles 2017-03-03 20:11:30 UTC
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.
Comment 10 Daniel Boles 2017-03-05 18:29:24 UTC
(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.