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.

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

core(without-javascript): allow noscript pages #5571

Merged
merged 2 commits into from
Jul 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion lighthouse-core/audits/without-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class WithoutJavaScript extends Audit {
static audit(artifacts) {
const artifact = artifacts.HTMLWithoutJavaScript;

if (artifact.value.trim() === '') {
// Fail pages that have empty text and are missing a noscript tag
if (artifact.bodyText.trim() === '' && !artifact.hasNoScript) {
return {
rawValue: false,
explanation: 'The page body should render some content if its scripts are not available.',
Expand Down
11 changes: 8 additions & 3 deletions lighthouse-core/gather/gatherers/html-without-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ const Gatherer = require('./gatherer');
function getBodyText() {
// note: we use innerText, not textContent, because textContent includes the content of <script> elements!
const body = document.querySelector('body');
return Promise.resolve(body ? body.innerText : '');
return Promise.resolve({
bodyText: body ? body.innerText : '',
hasNoScript: !!document.querySelector('noscript'),
});
}

class HTMLWithoutJavaScript extends Gatherer {
Expand All @@ -37,13 +40,15 @@ class HTMLWithoutJavaScript extends Gatherer {
// Reset the JS disable.
passContext.disableJavaScript = false;

const bodyText = await passContext.driver.evaluateAsync(`(${getBodyText.toString()}())`);
const expression = `(${getBodyText.toString()}())`;
const {bodyText, hasNoScript} = await passContext.driver.evaluateAsync(expression);
if (typeof bodyText !== 'string') {
throw new Error('document body innerText returned by protocol was not a string');
}

return {
value: bodyText,
bodyText,
hasNoScript,
};
}
}
Expand Down
20 changes: 17 additions & 3 deletions lighthouse-core/test/audits/without-javascript-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ describe('Progressive Enhancement: without javascript audit', () => {
it('fails when the js-less body is empty', () => {
const artifacts = {
HTMLWithoutJavaScript: {
value: '',
bodyText: '',
hasNoScript: false,
},
};

Expand All @@ -26,7 +27,8 @@ describe('Progressive Enhancement: without javascript audit', () => {
it('fails when the js-less body is whitespace', () => {
const artifacts = {
HTMLWithoutJavaScript: {
value: ' ',
bodyText: ' ',
hasNoScript: false,
},
};

Expand All @@ -38,7 +40,19 @@ describe('Progressive Enhancement: without javascript audit', () => {
it('succeeds when the js-less body contains some content', () => {
const artifacts = {
HTMLWithoutJavaScript: {
value: 'test',
bodyText: 'test',
hasNoScript: false,
},
};

assert.equal(withoutJsAudit.audit(artifacts).rawValue, true);
});

it('succeeds when the js-less body contains noscript', () => {
const artifacts = {
HTMLWithoutJavaScript: {
bodyText: '',
hasNoScript: true,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('HTML without JavaScript gatherer', () => {
disableJavaScript: true,
driver: {
evaluateAsync() {
return Promise.resolve('Hello!');
return Promise.resolve({bodyText: 'Hello!'});
},
},
};
Expand All @@ -41,15 +41,16 @@ describe('HTML without JavaScript gatherer', () => {
});

it('returns an artifact', () => {
const innerText = 'Hello!';
const bodyText = 'Hello!';
const hasNoScript = true;
return htmlWithoutJavaScriptGather.afterPass({
driver: {
evaluateAsync() {
return Promise.resolve(innerText);
return Promise.resolve({bodyText, hasNoScript});
},
},
}).then(artifact => {
assert.strictEqual(artifact.value, innerText);
assert.deepStrictEqual(artifact, {bodyText, hasNoScript});
});
});

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/results/artifacts/artifacts.json
Original file line number Diff line number Diff line change
Expand Up @@ -2520,6 +2520,6 @@
"value": false
},
"HTMLWithoutJavaScript": {
"value": "Do better web tester page\nHi there!\ntouchmove section\n "
"bodyText": "Do better web tester page\nHi there!\ntouchmove section\n "
}
}
2 changes: 1 addition & 1 deletion typings/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ declare global {
/** The hreflang and href values of all link[rel=alternate] nodes found in HEAD. */
Hreflang: {href: string, hreflang: string}[];
/** The page's document body innerText if loaded with JavaScript disabled. */
HTMLWithoutJavaScript: {value: string};
HTMLWithoutJavaScript: {bodyText: string, hasNoScript: boolean};
/** Whether the page ended up on an HTTPS page after attempting to load the HTTP version. */
HTTPRedirect: {value: boolean};
/** Information on size and loading for all the images in the page. */
Expand Down