GNOME Bugzilla – Bug 774695
GtkProgressbar needs full and empty classes
Last modified: 2016-12-03 17:23:12 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.
Created attachment 340276 [details] [review] Dynamically add/remove empty and full css classes on the trough node
Created attachment 340277 [details] [review] Make sure that empty progressbars are empty in Adwaita
Created attachment 340278 [details] Screenshot showing the before/after
Created attachment 340279 [details] [review] 3: Make sure progressbars always have a fill leve of >=1px
Review of attachment 340279 [details] [review]: I'd go for something like the following here &.empty progress { all: unset; }
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.
Adwaita theme bits pushed to master with commit eb5b8b22e2746aa5e85070ef5743b773fd0662b0, HC theme bits with commit cb1a349d17031beb766fd053c663f1c9d32cd2ef
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 https://bugzilla.xfce.org/show_bug.cgi?id=12942
Indeed, backporting this should be no risk and as Landry mentioned, many people have already noticed the problem.
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.
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).