Skip to content

Commit

Permalink
Fixes #80. Guard against IE11 focusing SVG. (#146)
Browse files Browse the repository at this point in the history
* Fixes #80. Guard against IE11 focusing SVG.

* Update test to check that focus-visible is not applied.
  • Loading branch information
robdodson authored Aug 4, 2018
1 parent effec50 commit 1756d66
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/focus-visible.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,17 @@ function init() {

/**
* Helper function for legacy browsers and iframes which sometimes focus
* elements like document and body.
* elements like document, body, and non-interactive SVG.
* @param {Element} el
*/
function isValidFocusTarget(el) {
if (
el &&
el !== document &&
el.nodeName !== 'HTML' &&
el.nodeName !== 'BODY'
el.nodeName !== 'BODY' &&
'classList' in el &&
'contains' in el.classList
) {
return true;
}
Expand Down
26 changes: 26 additions & 0 deletions test/fixtures/svg.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>svg focus fixture</title>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<style>
.js-focus-visible :focus:not(.focus-visible) {
outline: none;
}
.focus-visible {
outline: 2px solid red;
}
</style>
</head>
<body>
<button id="start">Start</button>
<button id="middle">
<svg id="icon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100" width="100" height="100">
<circle cx="50" cy="50" fill="red" r="50"></circle>
</svg>
</button>
<button id="end">End</button>
<script src="/dist/focus-visible.js"></script>
</body>
</html>
63 changes: 63 additions & 0 deletions test/specs/svg.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
const { fixture, FOCUS_RING_STYLE } = require('./helpers');
const { Key, By } = require('selenium-webdriver');
const expect = require('expect');
const driver = global.__driver;

// IE11 has a strange behavior where it will always focus an <svg> on the page.
// This test is to verify that we don't hit an error in this situation.
// See https://github.com/WICG/focus-visible/issues/80#issuecomment-383424156.
describe.only('svg focus', function() {

This comment has been minimized.

Copy link
@Agamnentzar

Agamnentzar Aug 4, 2018

should remove .only from here

This comment has been minimized.

Copy link
@robdodson

robdodson via email Aug 4, 2018

Author Collaborator
beforeEach(function() {
return fixture('svg.html');
});

it('should NOT apply .focus-visible if a non-interactive SVG is keyboard focused', async function() {
let actual;

// Tells Selenium to keep sending Tabs until the end element is reached.
async function tabUntil(body, end) {
let activeId;
let activeElement;

while (true) {
activeId = await driver.executeScript(
`return document.activeElement.id`
);

if (activeId) {
if (activeId === end) {
break;
}
// Only IE11 will stop here and focus the #icon element.
// If the element has focus, assert that :focus-visible has not been
// applied to it.
if (activeId === 'icon') {
actual = await driver.executeScript(`
return window.getComputedStyle(document.querySelector('#end')).outlineColor
`);
expect(actual).toNotEqual(FOCUS_RING_STYLE);
}
// Move focus to the next element.
// IE11's selenium driver won't move focus if we send it to body again
// so we need to send it to the activeElement.
activeElement = await driver.findElement(By.css(`#${activeId}`));
await activeElement.sendKeys(Key.TAB);
} else {
// Work around IE11 weirdness which sends focus to <html> first.
await body.sendKeys(Key.TAB);
}
}
}

let body = await driver.findElement(By.css('body'));
await body.click();
// Tabs through the document until it reaches the last element.
// In IE11 the non-interactive SVG, #icon will be focused.
// If it throws, then the test will fail.
await tabUntil(body, 'end');
actual = await driver.executeScript(`
return window.getComputedStyle(document.querySelector('#end')).outlineColor
`);
expect(actual).toEqual(FOCUS_RING_STYLE);
});
});

2 comments on commit 1756d66

@itsdouges
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey mate thanks for fixing this.

Just FYI you have an .only on your describe block!

@robdodson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Madou yep fixing it in #165, thanks for the heads up!

Please sign in to comment.