[GTK 3.20 Adwaita] Borders and separators too dark and thick

RESOLVED FIXED in Firefox 50

Status

()

Core
Widget: Gtk
RESOLVED FIXED
a year ago
9 months ago

People

(Reporter: carlosjosepita, Assigned: acomminos)

Tracking

(Blocks: 1 bug)

45 Branch
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

(Reporter)

Description

a year ago
Created attachment 8744538 [details]
2016-04-22-214307_1920x1080_scrot.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160412225428

Steps to reproduce:

Just run firefox (45 or 46b11). My gtk theme is adwaita. My os is arch.

 local/gtk2 2.24.30-1
    GObject-based multi-platform GUI toolkit (legacy)
local/gtk3 3.20.3-1
    GObject-based multi-platform GUI toolkit



Actual results:

Navigation bar, menu, find bar, etc. borders are too dark and thick. See the attached screenshot.
(Reporter)

Comment 1

a year ago
See https://bugzilla.mozilla.org/show_bug.cgi?id=1211892 for a related issue.
(Reporter)

Comment 2

a year ago
One main offender seems to be ThreeDShadow. For example, setting:

#nav-bar {
    --urlbar-border-color: #a2a2a1 !important;
}

makes the navigation bar looks good again.
(Reporter)

Comment 3

a year ago
This started to happen after a recent update (gtk or firefox update, I can't tell for sure). I've no firefox addons nor themes installed. My gtk configuration is pretty standard (just out-of-the-box adwaita).
Thanks for the bug report! CC'ing the folks who've been working on GTK stuff recently.

In the meantime:

(In reply to carlosjosepita from comment #3)
> This started to happen after a recent update (gtk or firefox update, I can't
> tell for sure).

To figure out what needs fixing, it would be helpful if you could narrow down whether it was a Firefox update or a GTK update that introduced the issue.  e.g. could you download an older version of Firefox, like v43 and/or v44 from here:
  https://ftp.mozilla.org/pub/firefox/releases/43.0/
  https://ftp.mozilla.org/pub/firefox/releases/44.0/
...and report back about whether it's affected?

(Note that before starting one of these, you'll need to quit out of your current Firefox sesion -- otherwise, when you try to start the old version, it'll just open a new window in the already-running Firefox process.  (The "-no-remote" command-line flag is an alternative way around this, if you're familiar with that.))

(You may also want to create a fresh Firefox profile when testing older versions, in case something breaks from the newer-->older-->newer version changes -- though that's pretty unlikely.)
Flags: needinfo?(carlosjosepita)
See Also: → bug 1211892
Summary: Borders and separators too dark and thick → [gtk3, Arch] Borders and separators too dark and thick
(Reporter)

Comment 5

a year ago
Ok, Daniel. Here is what I have:

1) Downgrading firefox to 44 has no effect regarding the issue.

2) Downgrading gtk3 from gtk3-3.18.9-1-x86_64.pkg.tar.xz to gtk3-3.18.8-1-x86_64.pkg.tar.xz fixes the problem (but I had to simultaneously downgrade firefox to 44 because of dependency problems. Nevertheless, both firefox 44 and 45 show thick borders against a newer gtk3).

So my conclusion is that the gtk3 upgrade was the culprit. I don't know what changed between 3.18.8 and 3.18.9 but it may give a hint to you.
(Reporter)

Comment 6

a year ago
I found out that the first arch package for ff 45 works against the older gtk and it doesn't show dark borders. So I can confirm that even some version of ff 45 is free of this problem, reinforcing the idea of a gtk3 upgrade being the cause.
(Reporter)

Comment 7

a year ago
Regarding the gtk3 packages above, sorry I made a mistake and I'm not able to edit the comment. The problematic jump is from:
  gtk3-3.18.9
to
  gtk3-3.19.12
at least with:
  firefox 45.0.1.

This make more sense because there were many adwaita changes:

https://git.gnome.org/browse/gtk+/log/?h=3.19.12&qt=grep&q=adwaita

Updated

a year ago
See Also: → bug 1267179

Updated

a year ago
See Also: bug 1267179
Duplicate of this bug: 1267179
Not sure that 1234158 is related, but it may turn out necessary.
Blocks: 1264079
Depends on: 1234158
Summary: [gtk3, Arch] Borders and separators too dark and thick → [GTK 3.20 Adwaita] Borders and separators too dark and thick
Flags: needinfo?(carlosjosepita)
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → bug 1269121
Looks like GTK 3.20 removed the addition of GTK_STYLE_CLASS_FRAME in gtk_frame_init as part of the transition to CSS nodes. We could add it back, the rules are still there- what do you think, Karl? It might be a useful stopgap, considering fetching the border colour is technically deprecated anyway.
Flags: needinfo?(karlt)
Interesting.
https://git.gnome.org/browse/gtk+/commit/?id=576028bdec90b27a42e8a65755dc0b2e235cdf5a
looks like it may have been trying to maintain support for foreign drawing.
I wonder whether that is intentional and widespread.

https://git.gnome.org/browse/gtk+/commit/?id=aa5dc38b0df3719a3407f8471c50b0355c113b5f
had already broken support for themes, settings, and Gecko.

(In reply to Andrew Comminos [:acomminos] from comment #10)
> It might be a useful stopgap,

I'm not so keen on adding a stop-gap, which will likely not be effective in other themes (if anyone would still dare to write one).

"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."

Seems we should get colors from the "border" subnode.

WidgetStyleCache.cpp has started providing that kind of thing.
I suspect it doesn't have support for detecting theme changes on 3.20 subnodes yet, but that's not a prerequisite for using it.

> considering fetching the border colour is technically deprecated anyway.

What exactly do you mean by "technically deprecated"?
Is there a replacement?
Flags: needinfo?(karlt)
(In reply to Karl Tomlinson (ni?:karlt) from comment #11)
> > considering fetching the border colour is technically deprecated anyway.
> 
> What exactly do you mean by "technically deprecated"?
> Is there a replacement?

The replacement for gtk_style_context_get_border_color (according to the GTK guys) would be use of the gtk_render APIs, which isn't possible in our case for nsLookAndFeel. It's in our best interest to stick with it for now, with our workaround for unico.

https://developer.gnome.org/gtk3/stable/GtkStyleContext.html#gtk-style-context-get-border-color

(In reply to Karl Tomlinson (ni?:karlt) from comment #11)
> Seems we should get colors from the "border" subnode.

https://blogs.gnome.org/mclasen/2015/11/20/a-gtk-update/ recommends construction a widget path for the subnode using gtk_widget_path_iter_set_object_name. That's probably the best solution, we just need to link against gtk_widget_path_iter_set_object_name at runtime for GTK 3.20 and then fetch the border colour from the border subnode.
(In reply to Andrew Comminos [:acomminos] from comment #12)
> https://developer.gnome.org/gtk3/stable/GtkStyleContext.html#gtk-style-
> context-get-border-color

"gtk_style_context_get_border_color has been deprecated since version 3.16 and should not be used in newly-written code.

Use gtk_render_frame() instead."

Ah, thanks.  Perhaps some day, we may just need to render to a small image and read pixels, but I don't know how to deal with border radius.

> (In reply to Karl Tomlinson (ni?:karlt) from comment #11)
> > Seems we should get colors from the "border" subnode.
> 
> https://blogs.gnome.org/mclasen/2015/11/20/a-gtk-update/ recommends
> construction a widget path for the subnode using
> gtk_widget_path_iter_set_object_name. That's probably the best solution, we
> just need to link against gtk_widget_path_iter_set_object_name at runtime
> for GTK 3.20 and then fetch the border colour from the border subnode.

https://dxr.mozilla.org/mozilla-central/rev/3461f3cae78495f100a0f7d3d2e0b89292d3ec02/widget/gtk/WidgetStyleCache.cpp#90
Duplicate of this bug: 1269121
See Also: bug 1269121

Comment 15

a year ago
Ok, so as explained in bug 1269121, for the color part it could be reversed and white borders with dark themed Gnome. I'll keep an eye on the problem for dark themed Gnome and just add to take care to:
- the line between UI and webpage because it's "cutting" it and create a clear separation between UI zone and scrollbar zone.
- the border that appeared around the menu when clicking on the sandwich menu, being very strong (more than before)
- separation lines in menus (both contextual ones on right click and on menus from menubar) which are very strongs too.
I just upgraded to Firefox 47 and started seeing this problem, with both gnome-breeze and breeze-gtk.  It also affects every single tab in Tree Style Tab's sidebar, making for an aesthetic catastrophe.

It didn't affect Firefox 46, even with the same version of GTK3.  Did something change in 47?

Nightly is the same, with the additional wrinkle that the scroll handle in scrollbars is completely invisible.
Workaround was to edit my GTK theme (which for me is /usr/share/themes/Breeze/gtk-3.0/gtk.css) and add:

    frame { border-color: #c0c2c4; }

That color's already set on various children and pseudoelements of a frame (including a border element, as mentioned above), but not on the frame itself, which is where Firefox looks to get a value for ThreeDShadow.  I'm guessing this is why gtk_style_context_get_border_color is deprecated.  :)
(In reply to Eevee (Lexy Munroe) [:eevee] from comment #16)
> I just upgraded to Firefox 47 and started seeing this problem, with both
> gnome-breeze and breeze-gtk.  It also affects every single tab in Tree Style
> Tab's sidebar, making for an aesthetic catastrophe.
> 
> It didn't affect Firefox 46, even with the same version of GTK3.  Did
> something change in 47?
> 
> Nightly is the same, with the additional wrinkle that the scroll handle in
> scrollbars is completely invisible.

I just installed the breeze theme on my fedora24 system and cannot duplicate the scroll handle issue using a currently nightly build.
(In reply to Eevee (Lexy Munroe) [:eevee] from comment #17)
> Workaround was to edit my GTK theme (which for me is
> /usr/share/themes/Breeze/gtk-3.0/gtk.css) and add:
> 
>     frame { border-color: #c0c2c4; }
> 
This hack is trickier to do for Adwaita, because it is the default theme and he gtk.css file is ignored.  There is a css file that according to it's comments only contains key-bindings, but it works fine if you add this line there.  Under fedora24 the file to change is:

/usr/share/themes/Default/gtk-3.0/gtk-keys.css
(Reporter)

Comment 20

a year ago
> there.  Under fedora24 the file to change is:
> 
> /usr/share/themes/Default/gtk-3.0/gtk-keys.css

It seems to me that the correct place to put this workaround is ~/.config/gtk-3.0/gtk.css

That said, I would like to also fix the too dark menu separators. Do you know what the appropriate css selector is?
Fixing the "frame" color also fixes Firefox's menu separators for me.  I don't have nearly-black lines anywhere now.
(Reporter)

Comment 22

a year ago
I found it: separator (https://developer.gnome.org/gtk3/stable/GtkSeparatorMenuItem.html)

This almost does the trick for me in Adwaita:

frame { border-color: #a2a2a1; }
separator { color: #a2a2a1; }

AFAICS there is one remaining detail to fix: the find bar (ctrl-f) upper border looks to thick. Any idea how to solve that?
(Reporter)

Comment 23

a year ago
I think the selector is something like findbar and the property maybe is clientTop (currently set to 2), but I'm not sure how to change it. Nevertheless, as I'm unable to go back to my previous installation after a system upgrade, and since I can't remember how the findbar top border looked like before that upgrade, I'm attaching a screenshot and asking you if yours looks the same as mine and if it's indeed intended to look like that.

https://postimg.org/image/6sw9wj4xd/
(Reporter)

Comment 24

a year ago
Created attachment 8770432 [details]
findbar top border

Updated

a year ago
Depends on: 1288696
Created attachment 8774356 [details]
Bug 1266914 - Use the border subnode for GtkFrame to fetch border colours.

Review commit: https://reviewboard.mozilla.org/r/66836/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66836/
Attachment #8774356 - Flags: review?(karlt)
Attachment #8774356 - Flags: review?(karlt) → review+
Comment on attachment 8774356 [details]
Bug 1266914 - Use the border subnode for GtkFrame to fetch border colours.

https://reviewboard.mozilla.org/r/66836/#review64384
Comment on attachment 8774356 [details]
Bug 1266914 - Use the border subnode for GtkFrame to fetch border colours.

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

Comment 28

a year ago
Pushed by acomminos@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7afc8055e8fb
Use the border subnode for GtkFrame to fetch border colours. r=karlt

Comment 29

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7afc8055e8fb
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Comment 30

a year ago
Created attachment 8775934 [details] [diff] [review]
cache style patch

Folks, there's a minor issue with this - the newly created style is not stored at widget cache as other css node based styles. It's no so big deal because it's used only once but better to fix that IMHO.
Attachment #8775934 - Flags: review?(karlt)
Comment on attachment 8775934 [details] [diff] [review]
cache style patch

Yes, that is what this function should do, thanks Martin.

I expect we might need MOZ_GTK_FRAME_BORDER for moz_gtk_frame_paint too, so keeping the style will be useful.
Attachment #8775934 - Flags: review?(karlt) → review+

Comment 32

a year ago
Comment on attachment 8775934 [details] [diff] [review]
cache style patch

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=69cff517689d

Comment 33

a year ago
Created attachment 8778832 [details] [diff] [review]
style cache patch for check-in

Updated

a year ago
Keywords: checkin-needed

Comment 34

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3803c63afd7e
cache newly created style, r=karlt
Keywords: checkin-needed

Comment 35

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3803c63afd7e
Assignee: nobody → andrew
Duplicate of this bug: 1315743

Updated

9 months ago
See Also: → bug 1315527

Updated

9 months ago
Depends on: 1315527
You need to log in before you can comment on or make changes to this bug.