Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: fix test for Safari #1557

Merged
merged 9 commits into from
May 16, 2019
Merged

test: fix test for Safari #1557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8683437
fix(tests): fix test for Safari, run Safari in CricleCI
straker May 9, 2019
499c3fd
add url to webkit bug
straker May 9, 2019
1b0a306
turn logging all the way one
straker May 9, 2019
b358057
just fix tests
straker May 9, 2019
656eaf2
update circle.yml
straker May 9, 2019
7564c92
just change circleci
straker May 9, 2019
7ef02e7
DRY webdriver builder setup, don't skip test for IE11
straker May 13, 2019
8e8ed3f
remove comments
straker May 14, 2019
cc86fca
Merge branch 'develop' into safari
straker May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ module.exports = function(grunt) {
langs = [''];
}

var webDriverTestBrowsers = ['firefox', 'chrome', 'ie', 'chrome-mobile'];
var webDriverTestBrowsers = [
'firefox',
'chrome',
'ie',
'chrome-mobile',
'safari'
];

process.env.NODE_NO_HTTP2 = 1; // to hide node warning - (node:18740) ExperimentalWarning: The http2 module is an experimental API.

Expand Down
36 changes: 21 additions & 15 deletions build/tasks/test-webdriver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ module.exports = function(grunt) {
/*
* Build web driver depends whether REMOTE_SELENIUM_URL is set
*/
function buildWebDriver(browser) {
var webdriver, capabilities;
async function buildWebDriver(browser) {
var capabilities;
var mobileBrowser = browser.split('-mobile');
if (mobileBrowser.length > 1) {
browser = mobileBrowser[0];
Expand All @@ -106,21 +106,27 @@ module.exports = function(grunt) {
};
}

var webdriver = new WebDriver.Builder()
.withCapabilities(capabilities)
.forBrowser(browser);

if (process.env.REMOTE_SELENIUM_URL) {
webdriver = new WebDriver.Builder()
.forBrowser(browser)
.withCapabilities(capabilities)
.usingServer(process.env.REMOTE_SELENIUM_URL)
.build();
} else {
webdriver = new WebDriver.Builder()
.withCapabilities(capabilities)
.forBrowser(browser)
.build();
webdriver.usingServer(process.env.REMOTE_SELENIUM_URL);
}

// @see https://github.com/SeleniumHQ/selenium/issues/6026
if (browser === 'safari') {
var safari = require('selenium-webdriver/safari');
var server = await new safari.ServiceBuilder()
.addArguments('--legacy')
.build()
.start();

webdriver.usingServer(server);
}

return {
driver: webdriver,
driver: webdriver.build(),
isMobile: mobileBrowser.length > 1
};
}
Expand All @@ -131,7 +137,7 @@ module.exports = function(grunt) {
grunt.registerMultiTask(
'test-webdriver',
'Task for launching Webdriver with options and running tests against options URLs',
function() {
async function() {
var driver;
var isMobile = false;
var done = this.async();
Expand Down Expand Up @@ -160,7 +166,7 @@ module.exports = function(grunt) {

// try to load the browser
try {
var webDriver = buildWebDriver(options.browser);
var webDriver = await buildWebDriver(options.browser);
driver = webDriver.driver;
isMobile = webDriver.isMobile;
// If load fails, warn user and move to the next task
Expand Down
8 changes: 7 additions & 1 deletion lib/commons/color/contrast.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ color.Color = function(red, green, blue, alpha) {
this.red = parseInt(match[1], 10);
this.green = parseInt(match[2], 10);
this.blue = parseInt(match[3], 10);
this.alpha = parseFloat(match[4]);

// alpha values can be between 0 and 1, with browsers having
// different floating point precision. for example,
// 'rgba(0,0,0,0.5)' results in 'rgba(0,0,0,0.498039)' in Safari
// when getting the computed style background-color property. to
// fix this, we'll round all alpha values to 2 decimal points.
this.alpha = Math.round(parseFloat(match[4]) * 100) / 100;
return;
}
};
Expand Down
9 changes: 9 additions & 0 deletions lib/commons/color/get-background-color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ color.filteredRectStack = function filteredRectStack(elm) {
const boundingStack = rectStack.shift();
let isSame;

// Safari v12.1 does not include labels as part of elementsFromPoint()
// if they wrap an input element (UNLESS the label has a background
// color). this results in two different rectStacks (since
// elm.getClientRects() returns two rects for the element) which
// returns null as isSame is false. we can fix this by adding in the
// missing label to the boundingStack before checking for isSame
// @see https://bugs.webkit.org/show_bug.cgi?id=197743
includeMissingElements(boundingStack, elm);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already happening in getBackgroundStack. I don't quite understand why you'd call this twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the includeMissingElements call in getBackgroundStack is not called since filteredRectStack returns null because it's missing the label. I tried removing the includeMissingElements in getBackgroundStack but that failed a lot of tests.


// iterating over arrays of DOMRects
rectStack.forEach((rectList, index) => {
if (index === 0) {
Expand Down
4 changes: 2 additions & 2 deletions test/checks/keyboard/page-no-duplicate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ describe('page-no-duplicate', function() {
function() {
var options = {
selector: 'footer',
nativeScopeFilter: 'main'
nativeScopeFilter: 'header'
};

var div = document.createElement('div');
div.innerHTML = '<main id="shadow"></main><footer></footer>';
div.innerHTML = '<header id="shadow"></header><footer></footer>';
div.querySelector('#shadow').attachShadow({ mode: 'open' }).innerHTML =
'<footer></footer>';
axe.testUtils.fixtureSetup(div);
Expand Down
58 changes: 30 additions & 28 deletions test/commons/text/form-control-value.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ describe('text.formControlValue', function() {
var formControlValue = axe.commons.text.formControlValue;
var fixtureSetup = axe.testUtils.fixtureSetup;
var fixture = document.querySelector('#fixture');
var isIE11 = axe.testUtils.isIE11;

function queryFixture(code, query) {
fixtureSetup(code);
Expand Down Expand Up @@ -120,33 +119,36 @@ describe('text.formControlValue', function() {
});
});

// This currently breaks in IE11
(isIE11 ? it.skip : it)(
'returns `` for non-text input elements',
function() {
fixtureSetup(
'<input type="button" value="foo">' +
'<input type="checkbox" value="foo">' +
'<input type="file" value="foo">' +
'<input type="hidden" value="foo">' +
'<input type="image" value="foo">' +
'<input type="password" value="foo">' +
'<input type="radio" value="foo">' +
'<input type="reset" value="foo">' +
'<input type="submit" value="foo">' +
'<input type="color" value="#000000">'
);
axe.utils
.querySelectorAll(axe._tree[0], '#fixture input')
.forEach(function(target) {
assert.equal(
nativeTextboxValue(target),
'',
'Expected no value for ' + target.actualNode.outerHTML
);
});
}
);
it('returns `` for non-text input elements', function() {
fixtureSetup(
'<input type="button" value="foo">' +
'<input type="checkbox" value="foo">' +
'<input type="file" value="foo">' +
'<input type="hidden" value="foo">' +
'<input type="image" value="foo">' +
'<input type="password" value="foo">' +
'<input type="radio" value="foo">' +
'<input type="reset" value="foo">' +
'<input type="submit" value="foo">' +
'<input type="color" value="#000000">'
);
axe.utils
.querySelectorAll(axe._tree[0], '#fixture input')
.forEach(function(target) {
// Safari and IE11 do not support the color input type
// and thus treat them as text inputs. ignore fallback
// inputs
if (target.actualNode.type === 'text') {
return;
}

assert.equal(
nativeTextboxValue(target),
'',
'Expected no value for ' + target.actualNode.outerHTML
);
});
});

it('returns the value of DOM nodes', function() {
fixture.innerHTML = '<input value="foo">';
Expand Down
13 changes: 8 additions & 5 deletions test/rule-matches/landmark-has-body-context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,14 @@ describe('landmark-has-body-context', function() {
(shadowSupport ? it : xit)(
'returns false for elements contained in a landmark in a shadow DOM tree',
function() {
var main = document.createElement('main');
var shadow = main.attachShadow({ mode: 'open' });
shadow.innerHTML = '<footer></fotoer>';

fixtureSetup(main);
// Safari has a bug in 12.0 that throws an error when calling
// attachShadow on <main>
// @see https://bugs.webkit.org/show_bug.cgi?id=197726
var article = document.createElement('article');
var shadow = article.attachShadow({ mode: 'open' });
shadow.innerHTML = '<footer></footer>';

fixtureSetup(article);
var vNode = axe.utils.querySelectorAll(axe._tree[0], 'footer')[0];
assert.isFalse(rule.matches(vNode.actualNode, vNode));
}
Expand Down