Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

#149 Be able to evaluate page with iframes #150

Merged
merged 6 commits into from
Jul 8, 2015
Merged

#149 Be able to evaluate page with iframes #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
88731e1
#149 Be able to evaluate page with iframes; disabling webSecurity in …
davidhsv May 2, 2015
2fbd434
#149 Adding unit test and fixing the opt_shadowRoot param.
davidhsv May 2, 2015
08b3be1
#149 Adding copy-dist...
davidhsv May 2, 2015
dc66bb7
Checking if contentDocument exists (I iframe can point to a 404 URL...)
davidhsv May 5, 2015
715c1e6
Merge remote-tracking branch 'origin/master' into #149
davidhsv Jun 13, 2015
83e5af1
Merge remote-tracking branch 'origin/master' into #149
davidhsv Jul 1, 2015
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
14 changes: 7 additions & 7 deletions dist/js/axs_testing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,11 +1288,11 @@ axs.properties.getTextFromAriaLabelledby = function(a, b) {
};
axs.properties.getTextFromHostLanguageAttributes = function(a, b, c, d) {
if (axs.browserUtils.matchSelector(a, "img") && a.hasAttribute("alt")) {
var e = {type:"string", valid:!0};
e.text = a.getAttribute("alt");
c ? e.unused = !0 : c = e.text;
b.alt = e;
}
var e = {type:"string", valid:!0};
e.text = a.getAttribute("alt");
c ? e.unused = !0 : c = e.text;
b.alt = e;
}
if (axs.browserUtils.matchSelector(a, 'input:not([type="hidden"]):not([disabled]), select:not([disabled]), textarea:not([disabled]), button:not([disabled]), video:not([disabled])') && !d) {
if (a.hasAttribute("id")) {
d = document.querySelectorAll('label[for="' + a.id + '"]');
Expand Down Expand Up @@ -1330,8 +1330,8 @@ axs.properties.getLastWord = function(a) {
axs.properties.getTextProperties = function(a) {
var b = {}, c = axs.properties.findTextAlternatives(a, b, !1, !0);
if (0 == Object.keys(b).length && ((a = axs.utils.asElement(a)) && axs.browserUtils.matchSelector(a, "img") && (b.alt = {valid:!1, errorMessage:"No alt value provided"}, a = a.src, "string" == typeof a && (c = a.split("/").pop(), b.filename = {text:c})), !c)) {
return null;
}
return null;
}
b.hasProperties = Boolean(Object.keys(b).length);
b.computedText = c;
b.lastWord = axs.properties.getLastWord(c);
Expand Down
8 changes: 8 additions & 0 deletions src/js/AuditRule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ axs.AuditRule.collectMatchingElements = function(node, matcher, collection,
}
}
}

//If it is a iframe, get the contentDocument
if (element && element.localName == 'iframe' && element.contentDocument) {
axs.AuditRule.collectMatchingElements(element.contentDocument,
matcher,
collection,
opt_shadowRoot);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is opt_shadowRoot necessary here? Is it required for collectMatchingElements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, it's a recursive method and I think it's necessary to always pass this atribute forward. For example, if we got a shadow down inside a iframe contentDocument, it will be needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense.

}
// If it is neither the parent of a ShadowRoot, a <content> element, nor
// a <shadow> element recurse normally.
var child = node.firstChild;
Expand Down
10 changes: 10 additions & 0 deletions test/js/audit-rule-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@
equal(matched.length, DIV_COUNT);
});

test("Iframe with simple DOM", function () {
var ifrm = document.createElement("IFRAME");
var container = document.getElementById('qunit-fixture');
container.appendChild(ifrm);
ifrm.contentDocument.body.appendChild(buildTestDom());
var matched = [];
axs.AuditRule.collectMatchingElements(container, matcher, matched);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this method mutating matched in memory? That doesn't seem ideal. Unrelated to this PR, but since we're in the neighborhood I thought I'd point it out. Not a nlocker for merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was justing copying the same behavior in others tests (in this same file). I don't like methods changing parameter's content too (bad smell).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, that's fine, thought I'd point it out as something we can improve in the future.

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 intended behaviour. Since this is a recursive method, we need an accumulator to keep track of matched elements - otherwise we'd be constantly having to combine partial results.

equal(matched.length, DIV_COUNT);
});

test("With shadow DOM with no content insertion point", function () {
var container = document.getElementById('qunit-fixture');
container.appendChild(buildTestDom());
Expand Down
3 changes: 3 additions & 0 deletions tools/runner/audit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ var page = require('webpage').create(),
system = require('system'),
url;

// disabling so we can get the document root from iframes (http -> https)
page.settings.webSecurityEnabled = false;

if (system.args.length !== 2) {
console.log('Usage: phantomjs audit.js URL');
phantom.exit();
Expand Down