GNOME Bugzilla – Bug 113802
Compliance problem for HighContrastInverse theme
Last modified: 2016-07-15 11:10:54 UTC
1)Invoke the theme capplet (Desktop preferences -> Theme) 2)Set the theme to 'HighContrastInverse' theme. 3)Notice in the workspace switcher applet that the demarcation between adjacent workspaces is not visible. The whole applet appears as one black strip. With a normal (Default) theme, the separation between workspaces is shown in white colored tinge. A similar demarcation for 'HighContrastInverse' theme would be very useful.
Since using the new High Contrast engine doesn't seem to fix this, I'm guessing the applet isn't picking the most sensible colour from the theme to draw the separator with... any ideas, Bill?
well, this does infer that workspace switcher isn't using the the normal draw_shadow routines from gdkstyle, since those now always draw contrasting borders with the new gtk-engine. To go further we'd need to look into the guts of the workspace switcher to see what it's really calling to do this drawing.
Workspace switcher applet in gnome-panel uses libwnck's pager and the changes with regard to this bug needs to be done in the lib., hence am transferring this bug to libwnck.
Libwnck doesn't use draw_shadow routines, I have done little changes to draw a border around each workspace rectangle and it fixes the issue by drawing a contrasting outline around each workspace for different themes. Am attaching the pacth and need your comments on it. Thanks.
Created attachment 16906 [details] [review] Patch as stated above.
Raj: I could be wrong, since I am looking at the patch out of context (i.e. not in the whole source file), but it looks as though you are mixing and matching states - using gc's from PRELIGHT intermixed with NORMAL. There's no guarantee that this will make sense for any theme in particular, so I think this is highly suspect. The basic problem, I think, is that dark_gc is of insufficient contrast from bg_gc for the inverse HC theme (for NORMAL - same is true for SELECTED in the 'normal' HC theme). We need to find a way other than just a line of dark_gc, to distinguish the wnck objects, IMO. I will think about this... by the way, I don't think changing dark_gc in the HC engine is the right thing to do either, since that would cause other weird visual effects.
Bill, > We need to find a way other than just a line of dark_gc, to distinguish the wnck objects, IMO. I find that by drawing a shadow around each workspace rectangle gives a clear distinction between them and when the inverse theme is selected it gives them contrasting borders there by giving a clear visibility.
Created attachment 16938 [details] [review] Patch as stated above.
Hi Raj: I don't know how Havoc will feel about the change to the look of the switcher, but the new patch does look "correct" in its use of theming and GTK_STATES, so I think it's good from our perspective.
Screenshot? Shouldn't the patch remove some code, not just add more drawing on top?
Hi Havoc: I have rewritten the patch to adjust the co-ordinates of the workspace rectangles there by the new shadow lines aren't overlapping them. Am attaching the patch and the corresponding screen shots. Thanks.
Created attachment 17054 [details] [review] Patch as stated above.
Created attachment 17055 [details] Applet with the patch applied in "Default Theme"
Created attachment 17056 [details] Applet with the patch applied in "HighContrastInverse Theme".
Created attachment 17057 [details] Applet with patch applied in "HighContrast Theme".
Created attachment 17058 [details] Applet without this patch in "HighContrastInverse Theme".
It looks bad because you're getting a double bevel all the way around the outside of the applet, due to the existing frame. You need each "edge" to have only a single 2-pixel bevel, rather than several pixels. i.e. basically I think you just need to add code to draw the lines between the workspaces, there's already a raised edge around the applet. I think gtk_paint_vline() may do this (does it draw a bevel? I think it does, maybe not) Or the other approach is to drop the code that draws the frame around the applet right now.
If we use drawing primitives as Havoc suggests, we need to make sure they are with respect to the correct, current GTK_STATE and not chosen "arbitrarily" to produce contrast with the HC theme. That means using either 'text[STATE]' or 'fg[STATE]' to draw these separators.
Havoc, Bill: I have worked out patches for both the above approaches suggested by Havoc. Approach A: ---------- > "to drop the code that draws the frame around the applet right now." The patch and the corresponding screenshots are attached below.
Created attachment 17172 [details] [review] Patch for approach 'A'.
Created attachment 17173 [details] Approach A: Screenshot for 'Default Theme'.
Created attachment 17174 [details] Approach A: Screen shot for "HighContrastInverse Theme".
Approach B: ---------- By drawing lines between workspaces using gtk_paint_hline/vline. In case of single row display only one line will be drawn to the rightmost side of each workspace rectangle. If the number of rows is greater than one then two lines one to the rightmost side and one to the bottom are drawn per workspace rectangle to distinguish it from the right and bottom workspaces respectively. Note: When number of rows is greater than one a small gap will be seen at the junction of a hline and vline of a workspace rectangle, it's because the lines hline and vline can't extend beyond the clip rectangle's width and height there by these two lines will never touch each other at the junction and thus creating a gap (like '_|'). (Hope I haven't confused:), pls. let me know if am missing something here.)
Created attachment 17176 [details] [review] Patch for approach 'B'.
Created attachment 17177 [details] Approach B: Screen shot for "Default Theme".
Created attachment 17178 [details] Approach B: Screen shot for "HighContrastInverse Theme".
Created attachment 17179 [details] Approach B: Screen shot for "HighContrast Theme".
Hi Raj: Thanks a lot for doing all this work, and making the screenshots available. This is very helpful. I like the visual results of approach B (though I have not read the patch for A, I confess) - and I expect Havoc will like the fact that the look of the window-list is more in keeping with its current look. However, I think option B has the disadvantage that the HighContrastInverse theme's separator lines may not provide quite enough contrast. I wonder if we can do anything to improve that, i.e. pick a color with a tiny bit more contrast against the near-black background? Actually I think we can fix this problem in the gtk-engine for HC themes, by overriding draw_vline and draw_hline. So I like 'B'. Havoc?
Marking AP2 to reflect a11y team's assessment of a11y impact.
Hi all: can we get approach 'B' committed so that I/we can start work on the accompanying bugfix for the HighContrast gtk-engine? thanks.
Am waiting for Havoc's comments!:)
B looks good, some nitpicks: The braces are wrong, should be on a line by themselves. If you have "foo (width - 2)" it should look like that, avoid "foo (width -2)" or "foo (width - 2 )" I believe the "2" subtracted from height should be "ythickness * 2" and the from width "xthickness * 2", except if GTK_SHADOW_NONE it should be zero. Also, you may need to subtract focus_width. See the size_request routine where x/ythickness are factored in for the frame outline, and the gtk_paint_shadow() call where focus_width is left around the outside of the shadow. I may have the details slightly wrong here but the point is the "2" doesn't seem like it should be hardcoded, so you might have a look at that. You can set xthickness, ythickness, and focus width in the gtkrc for testing. Thanks
Apologies for spam... marking as GNOMEVER2.3 so it appears on the official GNOME bug list :)
Rajkumar, could you update the patch according to Havoc's comments?
Raj, we're waiting for you ... :-)
Bill, I'll have a look at it :)
Apologies for spam-- ensuring Sun a11y team are cc'ed on all current a11y bugs. Filter on "SUN A11Y SPAM" to ignore.
Moving to right component. Sorry for the spam.
From comments it looks like people decided to go with B, so I'm marking the patch for A as commented on and due to Havoc's comments I'm marking the other patch as needs work.
Apologies for spam... ensuring Sun a11y folks are cc'ed on all current accessibility bugs.
*** Bug 335186 has been marked as a duplicate of this bug. ***
Created attachment 83565 [details] [review] updated patch according to havoc's comments I updated the patch according to the comments. Sorry that this has fallen victim of lazyness :-)
Kjartan: did you look if the patch needs changes because of the thickness/focus (as Havoc pointed to in comment #32)?
No, sorry I didn't have time to get to know the code well enough to do that.
Closing as obsolete.