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 774695 - GtkProgressbar needs full and empty classes
GtkProgressbar needs full and empty classes
Product: gtk+
Classification: Platform
Component: Widget: Other
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
Depends on:
Reported: 2016-11-18 22:34 UTC by Simon Steinbeiss
Modified: 2016-12-03 17:23 UTC
See Also:
GNOME target: ---
GNOME version: ---

Dynamically add/remove empty and full css classes on the trough node (2.27 KB, patch)
2016-11-18 22:47 UTC, Simon Steinbeiss
committed Details | Review
Make sure that empty progressbars are empty in Adwaita (1022 bytes, patch)
2016-11-18 22:47 UTC, Simon Steinbeiss
none Details | Review
Screenshot showing the before/after (15.82 KB, image/png)
2016-11-18 22:48 UTC, Simon Steinbeiss
3: Make sure progressbars always have a fill leve of >=1px (863 bytes, patch)
2016-11-18 22:53 UTC, Simon Steinbeiss
needs-work Details | Review

Description Simon Steinbeiss 2016-11-18 22:34:04 UTC
Currently there is a bug in either Gtk+ or Adwaita - I'd argue it depends on your point of view - that leads to 0% progressbars seeming to have a fill-level.

Briefly summarized, the issue is twofold:
1) The empty progress node still has a border, so themes that draw the border of the progress node end up having a 2px "leftover" when the progressbar is in fact totally empty. This can be misleading to users (e.g. when the progressbar is used for monitoring of network or IO performance), so it should be fixed somehow.

2) The issue does not only affect 0.00%, but also potentially 0.01% and above. Basically it causes the same visual glitch as in 1) if the width of the fill level is somewhere between 0px and 1px.

The proposed solution for 1) is a patch for gtkprogressbar.c (see attachment) which dynamically adds "empty" and "full" css classes on the trough node depending on the fill level. This enables themers to set "border: none;" and consequently avoid the misleading rendering described in 1).

The other issue 2), which I would describe not as a real bug, but as a visual glitch, can be mitigated by setting a "min-width: 1px" on the progress node for when the progressbar is not empty. Naturally this solution depends on the implementation of the patch for 1).

As this is my first patch for Gtk+ I ask for your indulgence. I tried my best to stick to coding style and to make my callback "fit in" with the rest of the code.
Comment 1 Simon Steinbeiss 2016-11-18 22:47:00 UTC
Created attachment 340276 [details] [review]
Dynamically add/remove empty and full css classes on the trough node
Comment 2 Simon Steinbeiss 2016-11-18 22:47:39 UTC
Created attachment 340277 [details] [review]
Make sure that empty progressbars are empty in Adwaita
Comment 3 Simon Steinbeiss 2016-11-18 22:48:08 UTC
Created attachment 340278 [details]
Screenshot showing the before/after
Comment 4 Simon Steinbeiss 2016-11-18 22:53:57 UTC
Created attachment 340279 [details] [review]
3: Make sure progressbars always have a fill leve of >=1px
Comment 5 Lapo Calamandrei 2016-11-21 15:09:07 UTC
Review of attachment 340279 [details] [review]:

I'd go for something like the following here

&.empty progress { all: unset; }
Comment 6 Lapo Calamandrei 2016-11-21 15:15:00 UTC
For the minimum lengh of the progressbar you can't just do like in attachment 340279 [details] [review], since you need to consider orientation, no need to use the not selector also.
Look at line 3177 the minimum lenght has to go in those structures and needs to be at least 3px, maybe 4, considering the border radius.
Comment 7 Lapo Calamandrei 2016-11-23 16:27:02 UTC
Adwaita theme bits pushed to master with commit eb5b8b22e2746aa5e85070ef5743b773fd0662b0, HC theme bits with commit 
Comment 8 Landry Breuil 2016-12-01 12:09:43 UTC
Nice work ! Can this be backported to 3.20 or 3.22 next dot releases ? Downstream we're getting plenty of duplicate bugs because of this, see
Comment 9 Simon Steinbeiss 2016-12-01 22:05:07 UTC
Indeed, backporting this should be no risk and as Landry mentioned, many people have already noticed the problem.
Comment 10 Daniel Boles 2016-12-03 16:34:30 UTC
As this seems both important and uncomplicated, I've tested and cherry-picked the 3 relevant commits I could see to gtk-3-22 - hope this is OK.
Comment 11 Daniel Boles 2016-12-03 17:23:12 UTC
And tested and pushed the same commits to gtk-3-20, as well as Matthias's update to widget-factory that lets this be tested (for horizontal ProgressBars at least).