No focus styling when navigating with keyboard from outside the web content about focus-visible HOT 36 CLOSED

Great thank you Ian! I may merge the fix if no one has any objections?

from focus-visible.

@robdodson @iandevlin I think I've found a much simpler solution for this issue. Have a look at the PR.

from focus-visible.

hm that's interesting. In the Safari example, since it's just input[type=text] I would imagine this line https://github.com/WICG/focus-ring/blob/gh-pages/src/focus-ring.js#L103 would kick in to apply the focus-ring class since the target should be in the whitelist. Will take a look later today. Thanks for the ticket!

from focus-visible.

My guess would be that the keyboard event is "swallowed"/consumed by the actual browser UI, so does not trigger the onKeyDown and therefore does not set the hadKeyboardEvent. don't think there's any way to catch this until the user actually does a keystroke once in the actual web content

[edit: actually, ignore that, it should still fire the focus event...]

from focus-visible.

actually, in the case of Chrome and reverse-tabbing to the last element in the page - as it's an <input type="file"> - this does apply. although the onFocus function is triggered when the input receives focus, the keyboard event happened in the UI, so hadKeyboardEvent is false at that point, and a file type of input is not in the whitelist, and the if (hadKeyboardEvent || focusTriggersKeyboardModality(e.target)) evaluates to false. so that's why when SHIFT+TABbing from address bar to that file chooser the focus is not applied.

from focus-visible.

I cannot reproduce this bug in Safari as reported. I'm testing in Safari Version 10.1 (12603.1.30.0.34) on Mac OS 10.12.4 (16E195).

from focus-visible.

Update: I was able to reproduce this providing the first element in the page is not a text input. Easily reproducible in both Safari and Chrome if you change the demo such that a <button> is the first focusable element in the page.

from focus-visible.

Some additional findings: this bug is exclusive to Safari and Chrome; IE and Firefox don't have this issue. The reason: neither Chrome or Safari fire a keydown event in advance of focus moving from the browser's location bar into the document. As a result, there's no means of determining if the initialfocus event was preceded by a keydown event.

One solution I tried was to changeup the logic to rely on a mousedown event listener vs. a keydown event listener; only apply the focus-ring class if the focus event was not preceded by mousedown event:

function onMouseDown(e) {
  mouseDownBeforeFocus = true;
}

function onFocus(e) {
  if (!mouseDownBeforeFocus || focusTriggersKeyboardModality(e.target))
    addFocusRingClass(e.target);
  mouseDownBeforeFocus = false;
}

While this solution does indeed solve this problem, as well as simplify the overall focus ring source code, it means the focus-ring class will be applied if focus is triggered programmatically via a call to focus(), which isn't something we want.

@robdodson @alice I think the best solution would be to see if we can just fix the bug in Chrome, no? If so, I'd be happy to file a bug.

from focus-visible.

Yeah, our earliest explorations had pointer modality being the "exception" like this, but we ran into that exact issue with programmatic focus.

I think filing a bug on Chrome makes sense. If you drop a link here I can chase it up, or I can file it.

from focus-visible.

@alice I'll file it then. I'll also need to file the same bug for Safari. Any idea where I'd go to do that? And should I loop James Craig in (or someone else)?

from focus-visible.

https://bugs.webkit.org/enter_bug.cgi would probably be the place to go for Safari/WebKit.

from focus-visible.

@alice I've just tested this behavior even more extensively to include Firefox for Mac and Chrome on Windows and ugh more inconsistencies:

• Firefox on Mac exhibits the same aforementioned "bug" in Safari and Chrome: no keydown event is fired when navigating from the location bar into the document via the Tab key. This leads me to believe this behavior is rooted in the windowing environment on the Mac, and as such, perhaps is something somehow intentional?
• Chrome on Windows (surprisingly) behaves as it does on the Mac, with the same "bug."

So, here's the complete breakdown:

Browsers that do fire keydown in advance of initial focus into the document:

• IE
• Firefox for Windows

Browsers that do not fire keydown in advance of initial focus into the document:

• Chrome for Mac and Windows
• Firefox for Mac
• Safari

Given all of these findings I'm wondering if you could do some more investigation on your end with the Chrome team regarding reason behind differences. For example: is Chrome doing something special to be consistent cross platform? Does the Chrome team have a hunch as to why this bug would exist consistently across all browsers on the Mac?

from focus-visible.

conceptually, i'm tending more towards the idea that IE/Firefox(Win) are a bit strange in this respect compared to the other browsers. the keypress clearly happened in the UI, not the document itself, so it's strange that they'd send a keydown to the document when that happened outside of the document (too lazy to test, but I'm assuming they're special-casing keypresses that resulted in getting into the document, and that they're not firing events for any arbitrary keystrokes that happened anywhere else in the UI).

so i'm wondering if the approach in the polyfill needs to be extended (perhaps keeping track Document.hasFocus() which i'm presuming would be false when the user's in the UI)?

from focus-visible.

@patrickhlauke so you think the behavior in Firefox for Windows is just a bug then, leaving IE simply as the outlier? I was thinking the difference between Firefox for Mac and Windows is a demonstration of how this behavior might be inherited from the platform. I have exactly 0% experience developing desktop apps, so take my observation as nothing more than that.

If the current behavior exhibited by Chrome (Mac + Win), Firefox for Mac and Safari is in fact intentional and not a bug, then I think we should just close out this issue as I cannot imagine how we would fix this given the current browser API + events. Now, if the focus event had a property indicating how focus was achieved.... then this would be an easy fix. In fact, it would even make implementing the :focus-ring pseudo polyfill pretty easy. Consider, for example this idea:

function onFocus(e) {
  if (e.source == 'keyboard') {
   e.target.classList.add('focus-ring'); 
  }
}

Where the source property could be either "keyboard," "mouse" or "api" (e.g. focus achieved by calling focus()).

from focus-visible.

Agreed, it would be great if the focus event indicated its source. However, I can't see that being standardised in any timely manner.

I agree that it would be weird to send a keypress event for a keypress which happened outside of web contents, but I think an argument could be made for it in this case.

In fact, I think it would work well with any eventual API to explain a focus event - a relatedEvent (like relatedTarget) would be more useful and flexible than a string/enum, and would require the keyboard event to exist.

from focus-visible.

I asked Cynthia Shelly if she remembered any details about the IE behaviour, and she said it's always been that way in IE, but wasn't sure whether it was deliberate, or whether Edge also supports it.

from focus-visible.

@alice yeah - I wasn't thinking we'd be able to get browsers to indicate the source of the focus event any time soon, was just proposing that as an idea.

from focus-visible.

What's the verdict here? Close this issue if we consider the variations in the browsers intentional and ok? Or consider IE's behavior correct, and therefore begin filing bugs in Chrome, Safari and Firefox for Mac.

from focus-visible.

My opinion hasn't changed :) I think it would make sense to file bugs.

from focus-visible.

what's more likely to happen quicker: browsers adopting the behavior of IE for consistency/web compat (though doubt any projects in the wild rely on the behavior, as otherwise this inconsistency would have likely been found sooner), or browser adopting proper native :focus-ring?

from focus-visible.

related, if filing bugs is chosen, probably worth documenting this at least in https://compat.spec.whatwg.org/

from focus-visible.

We're going to try an experiment. We'll listen for the document's blur event to tell when focus is leaving the page. At that point we can install a pointer event listener which will try to detect if the user clicked on the page when focus reenters. We'll also do this at onload time. If focus enters the page and the point event listener wasn't triggered then we'll assume it came from a keyboard.

¯_(ツ)_/¯

we'll update the thread with what we find 😸

from focus-visible.

Bad news: the order of mousedown/touchstart and focus events doesn't seem to be predictable - so you can sometimes get the focus event first, sometimes mousedown.

from focus-visible.

@alice wonder if any of these (manually compiled) event order results can help somehow? https://patrickhlauke.github.io/touch/tests/results/#mobile-tablet-touchscreen-events

from focus-visible.

@alice yep. I'm not surprised. I already went down this road while trying to solve for this. I thought I had a comment to that effect earlier in this thread.

from focus-visible.

@patrickhlauke I tested it myself and it seems to be genuinely inconsistent (in Chrome at least - I didn't check other browsers since one browser being inconsistent makes it unworkable).

@kloots Hm, doesn't seem like there was in this thread at least.

from focus-visible.

@kloots regarding your suggestions

changeup the logic to rely on a mousedown event listener vs. a keydown event listener; only apply the focus-ring class if the focus event was not preceded by mousedown event

You mention this approach indeed works, however it has the drawback that

focus-ring class will be applied if focus is triggered programmatically via a call to focus()

I believe this could be mitigated by 'listening in' on HTMLElement.focus()

roughly:

var originalHTMLElementFocus =  HTMLElement.prototype.focus;

HTMLElement.prototype.focus = function(){
  hadProgrammaticalFocus =true;
  originalHTMLElementFocus.apply(this, arguments);
}

Might this serve as a solution, or am i missing something?

from focus-visible.

I personally have mixed feelings about monkey patching focus() because it's such an important API

from focus-visible.

@liorur, I agree with @robdodson regarding no messing with the focus event implementation. But I see where you're coming from. I wonder if we could accomplish the same goal via a focusin event listener — assuming that focusin fires before focus when focus is triggered programmatically vs. a user-generated event.

from focus-visible.

FWIW alice and i tested it and in FF, Chrome, and Safari on Mac it seems to go 'focus', followed by 'focusin'. Not sure if that helps anyway but wanted to share that finding :)

from focus-visible.

@kloots @robdodson i do share your unease about using HTMLElement.focus() because its considered a bad practice to mess with the browser api (thought keep in mind we are attempting a polyfill for a future browser behavior, so it stands to reason that we will make use of the browser infrastructure), regradless i couldn't not think of a scenario that this approach breaks, i would be grateful if you could point me one.

from focus-visible.

@kloots @robdodson, Another way to mitigate this issue (though admittedly not resolving it), is to combine the keyboard and mouse events detection along with special behavior for the first and last focusable elements on the pages.
In essence:

• keyboard event + focus event => add focus ring
• mouse event + focus event => add focus ring only to whitelist
• focus event only => add focus ring only to whitelist (treat as if it was a mouse event)
• focus event only, specifically on first/last focusable elements => add focus ring (treat as if it was a keyboard event)

the outcome will be that this bug will no longer be:

"No focus styling when navigating with keyboard from outside the web content (or into iframe)"

but rather :

"focus styling needlessly appears on first or last focusable elements if they receive focus programmatically"

Which i believe to be a significant decrease in severity.

from focus-visible.

We just merged #34 which means we are no longer attempting to match .focus-ring if someone moves focus programmatically. Instead it only matches if the user moved by pressing the Tab or Shift-Tab keys. I think programmatic focus was the only thing holding back @kloots' suggestion in #15 (comment).

I consider this a pretty severe bug since someone relying on a keyboard is basically guaranteed to hit it. So I'd like to explore if we can create something along the lines of @kloots proposed solution that still feels like an accurate :focus-ring polyfill.

from focus-visible.

I may have a fix for this if any adventurous souls want to give it a try. It lives in the https://github.com/WICG/focus-ring/tree/intial-pointer-fixup branch. Note, I removed the classList polyfill cuz having to do a rollup build every time I wanted to test something was annoying. So you'll have to test it on modern(ish) browsers. I checked in Chrome, Firefox, Safari, and Edge 15 and it seems to work.

from focus-visible.

I should probably explain the fix...

I changed it to assume that hasKeyboardevent is initially true instead of false. Then I added listeners for any kind of fine-grained pointing device input:

document.addEventListener('mousemove', onInitialPointerMove);
document.addEventListener('mousedown', onInitialPointerMove);
document.addEventListener('mouseup', onInitialPointerMove);
document.addEventListener('pointermove', onInitialPointerMove);
document.addEventListener('pointerdown', onInitialPointerMove);
document.addEventListener('pointerup', onInitialPointerMove);
document.addEventListener('touchmove', onInitialPointerMove);
document.addEventListener('touchstart', onInitialPointerMove);
document.addEventListener('touchend', onInitialPointerMove);

Hearing any of those events will change hasKeyboardEvent to false, and also remove all of the event listeners. My thinking is that we might not be able to listen to keydown but there's a good chance we'll be able to hear a mousemove or something like that before the user clicks/focuses something.

When the window is blurred I restore the initialPointerMove listeners. This is to account for situations where you switch tabs and then come back. I'm not entirely sure if I need to do this but I figured I'd include it just in case.

from focus-visible.

@robdodson Tested this today on Windows in Chrome, Firefox, Edge, and Internet Explorer 11.

My test file (https://iandevlin.com/html5/focus-ring) has a skip link at the top that should appear only when it has focus, and when the page is loaded, focus is set via JS on the first navigation item (about).

Expected results:
When page loads focus is set on the about link but cannot be seen. When tab is pressed, focus should move to the "contact" link and be visible with a blue border.

Results:
Windows 10

• Firefox 15: works as expected
• Chome 61: works as expected
• Edge 40: focus is set on "about" but the next tab press causes focus to jump out of the webpage and to the browser itself. If I remove the setting of initial focus on the "about" link it works as expected. This seems to be an issue with Edge though and not focus-ring.js
• Internet Explorer 11: works as expected

Mac 10.12

• Firefox 56: works as expected
• Chrome 61: works as expected
• Safari 11: works as expected

from focus-visible.