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 685421 - Stock focus ring theming is not good enough
Stock focus ring theming is not good enough
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Class: GtkStyleContext
3.6.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 685416
 
 
Reported: 2012-10-03 17:46 UTC by Cosimo Cecchi
Modified: 2014-05-12 02:01 UTC
See Also:
GNOME target: 3.14
GNOME version: ---



Description Cosimo Cecchi 2012-10-03 17:46:03 UTC
The stock implementation of gtk_render_focus() in GtkThemingEngine only takes two style properties into account: focus-line-width and focus-line-pattern. It's not possible to:
- change the focus ring color
- change the focus ring line radius
- use a different style that is not either solid or dashed

There's also a couple of other style propreties that deal with focus, but they're left to each individual widget to be implemented:
- focus-padding: an additional padding between the focus line and the widget edge. Some widgets allocate this extra space.
- interior-focus: whether to draw the focus ring inside a widget or outside as an additional bevel. Some widgets read the value of this style property and change the size and position of the focus ring.

I think a rough plan to change this status quo would include:
- change gtk_render_focus() to call gtk_render_frame() internally: this would solve the first three problems mentioned in this bug report
- as e.g. in a GtkButton, gtk_render_focus() is called with the style context of the button, we then need a way for the theme to select the focus ring element. Possible ideas: 1) add a "focus-ring" style class on the context when rendering the focus ring - this has probably the downside of requiring high specificity selectors in the theme that override borders for e.g. buttons in different scenarios; 2) make it a region 3) make it so that every focusable widget style context has a "virtual" child with a specific style class.
- the focus ring by default would have the same width of the frame; the focus-padding style property could be replaced by setting a margin on the focus ring element - If we still want to support something like interior-focus, it could be possible to set a padding on the focus-ring element; this would make it bigger than the frame, and keep the element inside, but it would probably complicate a bit the rendering code to support it generically inside widgets.

Opinions?
Comment 1 Benjamin Otte (Company) 2012-10-03 18:16:51 UTC
What is missing from outline in the regular box model, and using that and borders with :focus in the CSS?

My solution is to stop using gtk_render_focus() completely, ignore all style properties and just use better CSS in the theme.
Comment 2 Cosimo Cecchi 2012-10-03 18:33:37 UTC
(In reply to comment #1)
> What is missing from outline in the regular box model, and using that and
> borders with :focus in the CSS?

The problem with outline is it misses the radius; you're right in that we could use outline-offset instead of margins and paddings in my previous example with frames, to decide the distance of the focus ring from the frame though.

> My solution is to stop using gtk_render_focus() completely, ignore all style
> properties and just use better CSS in the theme.

But :focus and the focus ring are two different things - e.g. a button can be of state focused with no focus ring displayed.
We would still need an element, a widget state or a style class specifically for the "I have a focus ring" state - I don't see any way around it unless we decide that outlines in GTK are *only* used for focus, and then the theme could just set outline-width etc in a generic selector and it would only be rendered when a focus ring is needed.
Comment 3 Benjamin Otte (Company) 2012-10-03 18:46:08 UTC
(In reply to comment #2)
> But :focus and the focus ring are two different things - e.g. a button can be
> of state focused with no focus ring displayed.
>
I think this is a bug. A widget that does not want to appear focused should not set :focus when rendering. Unless we think it's up to the theme to decide if a button should have a focus ring. But I think it's based on application state, no?
Comment 4 Cosimo Cecchi 2012-10-03 19:26:59 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > But :focus and the focus ring are two different things - e.g. a button can be
> > of state focused with no focus ring displayed.
> >
> I think this is a bug. A widget that does not want to appear focused should not
> set :focus when rendering. Unless we think it's up to the theme to decide if a
> button should have a focus ring. But I think it's based on application state,
> no?

What I mean is that right now the fact that a widget is of state focused doesn't imply it renders the focus ring (but it might still use a different border to indicate that is focused), because key navigation and default focus widgets are two different things.
Whether a focus ring will be drawn depends on the value returned by gtk_widget_has_visible_focus(), which in turn changes depending from the value of the gtk-visible-focus GtkSetting (which can also be set by themes, yes).

I think keeping such a distinction is worthwhile, because we want to hide the focus ring until key navigation is used - hence the separate element.
Comment 5 William Jon McCann 2014-01-23 21:51:33 UTC
What I think we agreed on on IRC today:

1. Make sure widgets respect CSS padding
2. Stop using focus-padding and focus-line-width
3. Fix CSS in adwaita to use padding instead of focus-padding and focus-line-width
4. Make sure widgets respect margins
5. Introduce outline-radius CSS
6. Make render_focus() draw the outline (instead of render_frame() ) so that it appears on top of content
7. Deprecate focus-line-pattern, focus-padding, focus-line-width

Does that sound right?
Comment 6 Benjamin Otte (Company) 2014-01-23 22:01:41 UTC
> 4. Make sure widgets respect margins

I think we can leave that out of this bug. It's something widgets should do, but I think it's not necessary to get this thing working.

Other than that: yes, that's a plan that should work.
Comment 7 Matthias Clasen 2014-01-24 18:51:45 UTC
Do we have a list of widgets that need changes / fixes before we can move forward with killing the focus drawing code in adwaita ?
Comment 8 Cosimo Cecchi 2014-05-01 16:53:37 UTC
The first part of this was implemented now in GTK+ master.

What remains to be done is:
* change the theme to use regular padding for widgets
* change widgets to not read focus-line-width/focus-padding
* deprecating the focus properties

I'll try to work on these items in the next couple days.
Comment 9 Cosimo Cecchi 2014-05-12 02:01:21 UTC
This is actually complete now from my point of view.
Let's open separate bugs if further improvements are needed.