Skip to content

Commit

Permalink
Assign globals in mock-dom module, instead of attaching everything to…
Browse files Browse the repository at this point in the history 
… jQuery
  • Loading branch information
@jdan
jdan committed Oct 3, 2015
1 parent 492be3f commit 048c873
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 101 deletions.
3 changes: 1 addition & 2 deletions plugins/shared/audit.js
View file
Expand Up @@ -43,8 +43,7 @@ function patchCollectMatchingElements() {
*/
$.axs.AuditRule.collectMatchingElements = function(node, matcher, collection,
opt_shadowRoot) {
// Only assign if node is an element (i.e. <p> vs <!-- comment -->)
if (node.nodeType === 1)
if (node.nodeType === Node.ELEMENT_NODE)
var element = /** @type {Element} */ (node);

if (element && matcher.call(null, element))
Expand Down
56 changes: 23 additions & 33 deletions test/alt-text-test.js
View file
Expand Up @@ -6,16 +6,10 @@ let assert = require("assert");
let mockDom = require("./mock-dom");

describe("Alt text plugin", function() {
let dom = null;
let plugin = null;

before(function(done) {
mockDom.createDom(function(domObj) {
// Assign `dom` to the object returned by createDom, which allows
// us to set the HTML of the virtual environment, query it, and
// close the window.
dom = domObj;

mockDom.createDom(function() {
let LinkTextPlugin = require("../plugins/alt-text");
plugin = new LinkTextPlugin();

Expand All @@ -24,27 +18,27 @@ describe("Alt text plugin", function() {
});

it("should label images without alt text", function() {
dom.setHTML(`
document.body.innerHTML = `
<img id="bad-img" src="test.jpg" />
`);
`;

plugin.run();

assert(dom.$("#bad-img").hasErrorLabel());
assert($("#bad-img").hasErrorLabel());
});

it("should not label images with alt text", function() {
dom.setHTML(`
document.body.innerHTML = `
<img id="good-img" src="test.jpg" alt="This is a description" />
`);
`;

plugin.run();

assert(!dom.$("#good-img").hasLabel());
assert(!$("#good-img").hasLabel());
});

it("should not label images with aria-labels", function() {
dom.setHTML(`
document.body.innerHTML = `
<div id="label">Hello</div>
<img id="img-with-aria-labelledby"
aria-labelledby="label"
Expand All @@ -53,44 +47,40 @@ describe("Alt text plugin", function() {
<img id="img-with-aria-label"
aria-label="This is a label"
src="test2.jpg" />
`);
`;

assert(!dom.$("img-with-aria-labelledby").hasLabel());
assert(!dom.$("img-with-aria-label").hasLabel());
assert(!$("img-with-aria-labelledby").hasLabel());
assert(!$("img-with-aria-label").hasLabel());
});

it("should not label hidden images without alt text", function() {
dom.setHTML(`
document.body.innerHTML = `
<div style="display:none">
<img id="hidden-bad-img" src="test.jpg" />
</div>
<img aria-hidden="true" id="aria-hidden-bad-img" />
`);
`;

plugin.run();

assert(!dom.$("#hidden-bad-img").hasLabel());
assert(!dom.$("#aria-hidden-bad-img").hasLabel());
assert(!$("#hidden-bad-img").hasLabel());
assert(!$("#aria-hidden-bad-img").hasLabel());
});

it("should label presentational images with warnings", function() {
dom.setHTML(`
document.body.innerHTML = `
<img id="empty-alt" src="test.jpg" alt="" />
<img id="presentational" role="presentation" src="test2.jpg" />
`);
`;

plugin.run();

assert(dom.$("#empty-alt").hasErrorLabel());
assert(dom.$("#empty-alt").hasClass("tota11y-label-warning"));
assert(/decorative/.test(dom.$("#empty-alt").expandedText()));

assert(dom.$("#presentational").hasErrorLabel());
assert(dom.$("#presentational").hasClass("tota11y-label-warning"));
assert(/decorative/.test(dom.$("#presentational").expandedText()));
});
assert($("#empty-alt").hasErrorLabel());
assert($("#empty-alt").hasClass("tota11y-label-warning"));
assert(/decorative/.test($("#empty-alt").expandedText()));

after(function() {
dom.destroy();
assert($("#presentational").hasErrorLabel());
assert($("#presentational").hasClass("tota11y-label-warning"));
assert(/decorative/.test($("#presentational").expandedText()));
});
});
26 changes: 8 additions & 18 deletions test/landmarks-test.js
View file
Expand Up @@ -6,16 +6,10 @@ let assert = require("assert");
let mockDom = require("./mock-dom");

describe("Landmarks plugin", function() {
let dom = null;
let plugin = null;

before(function(done) {
mockDom.createDom(function(domObj) {
// Assign `dom` to the object returned by createDom, which allows
// us to set the HTML of the virtual environment, query it, and
// close the window.
dom = domObj;

mockDom.createDom(function() {
let LandmarksPlugin = require("../plugins/landmarks");
plugin = new LandmarksPlugin();

Expand All @@ -24,31 +18,27 @@ describe("Landmarks plugin", function() {
});

it("should label tags with a role", function() {
dom.setHTML(`
document.body.innerHTML = `
<div id="main-div" role="main">
Hello, world!
</div>
`);
`;

plugin.run();

assert(dom.$("#main-div").hasLabel());
assert(dom.$("#main-div").labelText() === "main");
assert($("#main-div").hasLabel());
assert($("#main-div").labelText() === "main");
});

it("should not label tags without a role", function() {
dom.setHTML(`
document.body.innerHTML = `
<div id="main-div">
Hello, world!
</div>
`);
`;

plugin.run();

assert(!dom.$("#main-div").hasLabel());
});

after(function() {
dom.destroy();
assert(!$("#main-div").hasLabel());
});
});
60 changes: 25 additions & 35 deletions test/link-text-test.js
View file
Expand Up @@ -6,16 +6,10 @@ let assert = require("assert");
let mockDom = require("./mock-dom");

describe("Link text plugin", function() {
let dom = null;
let plugin = null;

before(function(done) {
mockDom.createDom(function(domObj) {
// Assign `dom` to the object returned by createDom, which allows
// us to set the HTML of the virtual environment, query it, and
// close the window.
dom = domObj;

mockDom.createDom(function() {
let LinkTextPlugin = require("../plugins/link-text");
plugin = new LinkTextPlugin();

Expand All @@ -24,67 +18,67 @@ describe("Link text plugin", function() {
});

it("should not label descriptive links", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#" id="good-link">
This is a descriptive link
</a>
`);
`;

plugin.run();

assert(!dom.$("#good-link").hasLabel());
assert(!$("#good-link").hasLabel());
});

it("should label unclear links", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#" id="bad-link">Click here</a> to learn more about
dinosaurs. You can also <a id="better-link" href="#">
check out my blog</a>.
`);
`;

plugin.run();

assert(dom.$("#bad-link").hasLabel());
assert(!dom.$("#better-link").hasLabel());
assert($("#bad-link").hasLabel());
assert(!$("#better-link").hasLabel());
});

it("should consider alt text as link text", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#" id="logo-link">
<img src="/images/logo.png" alt="Our logo" />
</a>
`);
`;

plugin.run();

// TODO: Test the extended error messages for the presence of "alt"
assert(!dom.$("#logo-link").hasLabel());
assert(!$("#logo-link").hasLabel());
});

it("should fail on image links with unclear alt text", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#" id="bad-image-link">
<img src="/images/banner.png" alt="Click here" />
</a>
`);
`;

plugin.run();

assert(dom.$("#bad-image-link").hasLabel());
assert($("#bad-image-link").hasLabel());
});

it("should fail on empty links", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#" id="empty-link"></a>
`);
`;

plugin.run();

assert(dom.$("#empty-link").hasLabel());
assert($("#empty-link").hasLabel());
});

it("should pass on empty links with aria-labels", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#"
aria-label="this is a detailed description"
id="empty-link-with-label">
Expand All @@ -95,16 +89,16 @@ describe("Link text plugin", function() {
id="empty-link-with-labelledby">
</a>
<span id="link-description">This is a detailed description</span>
`);
`;

plugin.run();

assert(!dom.$("#empty-link-with-label").hasLabel());
assert(!dom.$("#empty-link-with-labelledby").hasLabel());
assert(!$("#empty-link-with-label").hasLabel());
assert(!$("#empty-link-with-labelledby").hasLabel());
});

it("should fail on links with unclear aria-labels", function() {
dom.setHTML(`
document.body.innerHTML = `
<a href="#"
aria-label="click here"
id="empty-link-with-unclear-label">
Expand All @@ -117,15 +111,11 @@ describe("Link text plugin", function() {
This is a longer description that will not be picked up
</a>
<span id="link-description">Click here</span>
`);
`;

plugin.run();

assert(dom.$("#empty-link-with-unclear-label").hasLabel());
assert(dom.$("#empty-link-with-unclear-labelledby").hasLabel());
});

after(function() {
dom.destroy();
assert($("#empty-link-with-unclear-label").hasLabel());
assert($("#empty-link-with-unclear-labelledby").hasLabel());
});
});
23 changes: 12 additions & 11 deletions test/mock-dom/index.js
View file
Expand Up @@ -35,6 +35,17 @@ exports.createDom = function(callback) {
html: "",
src: [axsSrc, jquerySrc, jqueryExtSrc],
done: function(errors, window) {
// Expose some fields from `window` onto the global namespace
//
// TODO: Currently we add fields here (see "Node") as we need
// them, but this may prove tricky to maintain.
global.__proto__ = {

This comment has been minimized.

Sign in to view

Copy link
@rileyjshaw

rileyjshaw Oct 6, 2015

Contributor

This seems kinda sketchy. I tend to avoid __proto__ so my experience is limited, but I thought this sort of reassignment was discouraged? You know better than I do.

I'm lacking a lot of context here, but is it possible to avoid using __proto__ altogether? global = window sounds too easy... but... something?

This comment has been minimized.

Sign in to view

Copy link
@jdan

jdan Oct 6, 2015

Author Owner

This is a super hack, and nearly the same as me simply declaring:

document = window.document;
$ = window.jQuery;

etc without the var keyword. Difference is that I won't be overriding any existing variables because I'm throwing them in the prototype.

__proto__ is deprecated in ES5 which is why this code is probably smelly, but in ES6 it's legit! jshint/jshint#2371. Looks like Object.setPrototypeOf() is also a thing? http://www.ecma-international.org/ecma-262/6.0/index.html#sec-object.setprototypeof
document: window.document,
$: window.jQuery,
axs: window.axs,
Node: window.Node,
};

// Overwrite the default module loader.
//
// Here we intercept `require()` calls to stub out the annotations
Expand All @@ -61,17 +72,7 @@ exports.createDom = function(callback) {
}
};

callback({
setHTML(html) {
window.document.body.innerHTML = html;
},

destroy() {
window.close();
},

$: window.jQuery,
});
callback();
}
});
};
2 changes: 0 additions & 2 deletions test/mock-dom/jquery-extensions.js
View file
Expand Up @@ -5,8 +5,6 @@
* This file will be loaded into test documents by the mock-dom utility.
*/

var $ = window.jQuery;

$.fn.hasLabel = function() {
return !!$(this).data("has-label");
};
Expand Down

0 comments on commit 048c873

Please sign in to comment.