When click on a <input type="button" />, <button /> or an <a href="..." />, the browser should fire the focusin event since these elements does get focus. If no elements found to be focusable, then either the Document or the BODY should fire the focusin event. This is working on Firefox and IE, but not on WebKit browsers.
Created attachment 40659 [details] patch
Comment on attachment 40659 [details] patch Although this patch gives the right behavior I think we need to be a bit careful here. With this patch links gets the focus outline when the user clicks them. This is correct when it comes to CSS because we have something like :link:focus { outline: ...}. However, I doubt that we want this heavy visual clutter on links by default. You can remove isMouseFocusable from HTMLAnchorElement now since all it does is call super. Also, you need to include a layout test with your patch.
(In reply to comment #2) Thanks for the review. I put some of my thoughts below. > (From update of attachment 40659 [details]) > Although this patch gives the right behavior I think we need to be a bit > careful here. > > With this patch links gets the focus outline when the user clicks them. This is > correct when it comes to CSS because we have something like :link:focus { > outline: ...}. However, I doubt that we want this heavy visual clutter on links > by default. As it affects the click behavior not the hover behavior, the performance degrade is probably ok. Other browsers, such as Firefox, also support this. > > You can remove isMouseFocusable from HTMLAnchorElement now since all it does is > call super. Yes, will do. > > Also, you need to include a layout test with your patch. I will work on that.
Created attachment 40824 [details] patch with test case
Comment on attachment 40824 [details] patch with test case I don't think we want this behavior on the Mac. Let me find a related bug with more explanation.
(In reply to comment #5) > (From update of attachment 40824 [details]) > I don't think we want this behavior on the Mac. Let me find a related bug with > more explanation. This actually fixes web compat issues. I think the main problem is the visual clutter the focus outline introduces
This is very similar to https://bugs.webkit.org/show_bug.cgi?id=22261. You should read some of the discussion there. This particular patch addresses the issue described in https://bugs.webkit.org/show_bug.cgi?id=18425. We should probably dupe some of these bugs.
> This actually fixes web compat issues. What are the sites that get fixed? I agree with Adele that we have way too many bugs tracking this.
Yes, they are very similar bugs and similar fixes. With all this in mind, I am thinking if we can do #if PLATFORM(MAC) if (isLink()) return false; #endif // Allow tab index etc to control focus. return HTMLElement::isMouseFocusable(); However, personally, I don't feel drawing the outline is a big performance overhead. The drawing happens only if the user click the link. Comparing to the time spent on human interaction, the drawing is negligible. On the other hand, it could even be a positive thing: the user gets feedback on what he/she clicked and even is able to stop the load if the link is accidentally clicked. Well, this is just my two cents.
It seems this bug 26856 is a superbug of 22261 and 18425. My patch fixes 18425 only. And Viatcheslav Ostapenko's patch fixes 22261.
I'm not concerned about performance. My concern is about the visual clutter. All other browsers allows the user to use the mouse to focus anchor elements, buttons and inputs. The difference is they have a more subtle focus outline than WebKit does so it is not so visually distracting. I think it is clear we should do this on all platforms. What we might want to do is to come up with a solution where it does not show the focused state when the user uses the mouse to focus an element. IE7 used to do this but it wasn't compatible with CSS so they always show the focus state in IE8, no matter how the element received focus.
(In reply to comment #11) > I'm not concerned about performance. My concern is about the visual clutter. > > All other browsers allows the user to use the mouse to focus anchor elements, > buttons and inputs. The difference is they have a more subtle focus outline > than WebKit does so it is not so visually distracting. > > I think it is clear we should do this on all platforms. What we might want to > do is to come up with a solution where it does not show the focused state when > the user uses the mouse to focus an element. IE7 used to do this but it wasn't > compatible with CSS so they always show the focus state in IE8, no matter how > the element received focus. I thought it was the performance caused by visual clutter. Understood. So what we want to come up is not to render/paint the visual effect when mouse-focused but all other things, such as fire events, should still work.
Created attachment 41053 [details] apply change only to non-Mac platform
Comment on attachment 41053 [details] apply change only to non-Mac platform Having FAIL in the platform/mac/ expectation seems bad to me.
(In reply to comment #14) > (From update of attachment 41053 [details]) > Having FAIL in the platform/mac/ expectation seems bad to me. As Erik and I discussed through IRC, it seems ok at the moment. The special expectation will be removed once the final solution is provided to Mac.
Comment on attachment 41053 [details] apply change only to non-Mac platform We should keep discussing the Mac behavior. Maybe we could turn it on as a trial at some point to see how it feels in practice.
Comment on attachment 41053 [details] apply change only to non-Mac platform Clearing flags on attachment: 41053 Committed r49619: <http://trac.webkit.org/changeset/49619>
All reviewed patches have been landed. Closing bug.
Updated Windows results were committed in http://trac.webkit.org/changeset/49667 and http://trac.webkit.org/changeset/49640.
I think we need to roll this back until bug 30523 is fixed. If rollback is not acceptable this needs to be exposed in the webkit API.
Created attachment 41971 [details] Make links mouse focusable only on GTK and QT only
(In reply to comment #21) > Created an attachment (id=41971) [details] > Make links mouse focusable only on GTK and QT only The test case change in this patch will break Qt, won't it?
(In reply to comment #22) > (In reply to comment #21) > > Created an attachment (id=41971) [details] [details] > > Make links mouse focusable only on GTK and QT only > > The test case change in this patch will break Qt, won't it? Yup, adding a new expectation to platform/qt/
Created attachment 41977 [details] Updated test expectations
Created attachment 41978 [details] Updated test expectations
I'm confused by the result output being: PASS FAIL Is this test intended to partially fail?
Created attachment 41991 [details] Added GTK and QT specific tests
Comment on attachment 41991 [details] Added GTK and QT specific tests Lets make this a single test, and check in separate results for the separate platforms, as you original suggested. LGTM. Fix it as you land.
Created attachment 42149 [details] Changed test to not say PASS/FAIL and only make the expectations platform specific
Comment on attachment 42149 [details] Changed test to not say PASS/FAIL and only make the expectations platform specific ChangeLog is wrong, otherwise this looks fine.
Created attachment 42159 [details] Fixed issues with tabIndex and contentEditable and expanded the tests to cover these
Comment on attachment 42159 [details] Fixed issues with tabIndex and contentEditable and expanded the tests to cover these It's a bit strange to see this in the results: Anchor with tab index 6 7 Anchor without tab index 8 9 Link without tab index 10 11 Link with tab index 12 13 Link with contentEditable 14 15 Link inside contentEditable it's just noise. For bonus points you could wrap it in a div and hwne running under DRT remove it at the end of the test. 74 // Make sure that links with tabIndex or contentEditable are still focusable. Maybe something more like: // Only allow links with a tabIndex or contentEditable to be mouse-focusuable. the "make sure" comment only makes sense given the previous code. :) In general this looks fine though.
Created attachment 42163 [details] Took care of Eric's comments
Comment on attachment 42163 [details] Took care of Eric's comments Works for me.
Comment on attachment 42163 [details] Took care of Eric's comments Clearing flags on attachment: 42163 Committed r50315: <http://trac.webkit.org/changeset/50315>
This is only working as expected on GTK and QT. We need to add support for a -webkit-focusring pseudo class (bug 30523) before we can make links mouse focusable again.
With r50315, any time you cmd-click on someone's profile pic on Facebook, you focus the link around the photo. Looks pretty terrible.
Facebook is now setting the tabindex to -1 on the anchor element which makes it mouse focusable so they are getting what they are asking for. Seriously though, I think bug 30523 is the only sane way forward.
The same change caused this regression: https://bugs.webkit.org/show_bug.cgi?id=38548
It doesn't seem practical to rely on developers adopting a new pseudo class in the future to fix the problem. Is there some intermediate step we should take to fix Facebook and UW in the short term? Should we consider rolling this change out for Mac? (In reply to comment #39) > Facebook is now setting the tabindex to -1 on the anchor element which makes it > mouse focusable so they are getting what they are asking for. Seriously though, > I think bug 30523 is the only sane way forward.
The UW bug does is not about the look. It is breaking because the link gets focused. Links are mouse focusable in all other browsers so the UW site must be doing some WebKit specific thing to cause this. I'll dissect the UW site tomorrow and see why they get this behavior. I think it might be acceptable to make links non mouse focusable (ignoring tabindex) again but only temporary. If the site declares that they want links to be focusable we should respect that. The issue with the focus ring is different. If we implement focus right correctly it would not require web sites to change anything. The difference is just that we would not show the focus ring on links when the user mouse focuses them, only when they use the keyboard to focus them.
(In reply to comment #42) > The UW bug does is not about the look. It is breaking because the link gets > focused. Links are mouse focusable in all other browsers so the UW site must be > doing some WebKit specific thing to cause this. I just updated the UW bug with various reductions and pieces of information that attempt to show that there is not WebKit-specific code in the UW test. Steven and I attached a few test cases that fail in WebKit even though they work in Firefox.
See also: bug 26856.
Sorry, discussion above already has more and better links to related bugs.
*** Bug 55975 has been marked as a duplicate of this bug. ***
*** Bug 92029 has been marked as a duplicate of this bug. ***
Is there a plan to revive this bug and finally address it? I ask because the bug I was part of (about radio buttons) was just marked as a dupe and merged into this one, but here this bug appears dormant for 11 years. Also, reading this thread, I find it unusual that the original objection was that elements which are focused "look bad" if there is some visual indication they are focused. I'm fairly certain that A11Y guidelines strongly push for elements which are focused to be visually obvious that they are focused. The fact that someone doesn't like what it looks like for a link or radio button to be focused is a CSS styling question, not a functionality question. When devs remove the `outline` on a focused element (probably at the request of some client or designer) and don't replace it with some other visual indication -- I prefer using an inner box shadow to make an element sort of "glow" -- that omission is pretty universally considered a bad A11Y practice, as far as I can tell. ---- I had long since forgotten that back in 2012 I was participating in that other bug about focus not working on radio buttons, and was only reminded when I saw the email indicating the dupe/merge here. But I fairly recently had this same problem in a PWA I built. I created a click handler for all buttons, checkboxes, and radio buttons in my app, that forces the element to be focused, so that mouse clicks or taps cause my CSS :focus styling to be applied in safari the same as it was automatically being applied in other browsers/platforms. So yeah, this is STILL a bug that code is still having to work-around.
Is there any difference between this and bug 22261 at this point? Seems like another dupe to me.
(In reply to Alexey Proskuryakov from comment #49) > Is there any difference between this and bug 22261 at this point? Seems like > another dupe to me. Pretty much.
*** This bug has been marked as a duplicate of bug 22261 ***