The focus ring disappears on some Internet pages, for example https://www.google.com/accounts/ServiceLogin : the focus ring disappears after passing the "Sign in" button and appears again at the "create one for free" link after a few tabs. The analyze of the page's source has given a simple test case, the focus ring is not shown on the "bad" page. The bug appears in the gtk branch built under FreeBSD (e.g. git f676a7ee078a079bc3bd9781d4d7c9ca5fd1d79c).
Created attachment 23100 [details] correct behavior
Created attachment 23101 [details] incorrect behavior
This seems to be due to the focus ring function in GraphicsContextCairo relying on the GraphicsContext to paint the focus ring, whereas other ports just use a platform focus ring paint function.
Created attachment 23684 [details] Fix focus ring painting
Comment on attachment 23684 [details] Fix focus ring painting This seems like the wrong fix. Why would Gtk want to ignore the color parameter and use the stroke color instead? Shouldn't whatever callers be fixed to pass the strokeColor() in the necessary cases for gtk? I'm just not even sure why that change would be needed. This seems like the wrong layer to abstract at.
(In reply to comment #5) > (From update of attachment 23684 [details] [edit]) > This seems like the wrong fix. Why would Gtk want to ignore the color > parameter and use the stroke color instead? Shouldn't whatever callers be > fixed to pass the strokeColor() in the necessary cases for gtk? I'm just not > even sure why that change would be needed. This seems like the wrong layer to > abstract at. > The color passed in always seems to be the same which isn't what we want. It looks OK when the focus ring is rounded, thick and transparent, but the GTK+ focus ring is a single pixel dotted rectangle which usually has the text/foreground color, and is used for anchors as well as controls. Not sure how to get the desired color passed in here except by using the stroke or fill colors. Any suggestions?
Don't we use a special keyword for the focus ring color? Doesn't the value of that color come from the RenderTheme?
(In reply to comment #7) > Don't we use a special keyword for the focus ring color? Doesn't the value of > that color come from the RenderTheme? > I didn't notice a way to adjust the appearance of focused anchor elements (the most common user of drawFocusRing() since RenderTheme handles all controls itself). Looking at RenderTheme/RenderStyleConstants.h the EAppearance values are mostly concerned with widgets/controls which doesn't seem to help.
(In reply to comment #7) > Don't we use a special keyword for the focus ring color? Doesn't the value of > that color come from the RenderTheme? > I missed the question! Yes, the color does come from RenderTheme but it's constant, whereas the GTK+ focus ring works best when it either uses the current foreground color of the context (ie. matching the color of the text), or otherwise contrasts with the background it's painted on. (In older applications, the focus ring would XOR, but still was never stuck to a single color.)
(In reply to comment #9) > (In reply to comment #7) > > Don't we use a special keyword for the focus ring color? Doesn't the value of > > that color come from the RenderTheme? > > > > I missed the question! Yes, the color does come from RenderTheme but it's > constant, whereas the GTK+ focus ring works best when it either uses the > current foreground color of the context (ie. matching the color of the text), > or otherwise contrasts with the background it's painted on. (In older > applications, the focus ring would XOR, but still was never stuck to a single > color.) > Indeed, a little testing shows that the value of strokeColor() doesn't _always_ contrast with the background, so the technique in my patch isn't perfect but passable. Any thoughts?
Don't we just need to support invert when painting outlines? http://www.w3.org/TR/2004/CR-css3-ui-20040511/#outline-color0 That seems to be the bug here.
If you want to use the current foreground color, then currentColor is the right keyword. Not sure we support that yet though. Either way I think the CSS spec covers what you need. We just have to add the right support for it.
Created attachment 23771 [details] Fix focus ring painting, take two Fixed to use the color passed in to drawFocusRing()
Comment on attachment 23771 [details] Fix focus ring painting, take two >+ // We prefer the stroke color for focus rings in GTK+. >+ setColor(cr, color); Above comment isn't necessary any more, of course.
Comment on attachment 23771 [details] Fix focus ring painting, take two Seems fine, r=me
Landed in r37521.