core(without-javascript): allow noscript pages

GoogleChrome · Jun 27, 2018 · 8c1d74e · 8c1d74e
1 parent 9c5b76c
commit 8c1d74e
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 12 deletions.
diff --git a/lighthouse-core/audits/without-javascript.js b/lighthouse-core/audits/without-javascript.js
@@ -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.',

diff --git a/lighthouse-core/gather/gatherers/html-without-javascript.js b/lighthouse-core/gather/gatherers/html-without-javascript.js
@@ -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 {
@@ -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,
     };
   }
 }

diff --git a/lighthouse-core/test/audits/without-javascript-test.js b/lighthouse-core/test/audits/without-javascript-test.js
@@ -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,
       },
     };
 
@@ -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,
       },
     };
 
@@ -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,
       },
     };
 

diff --git a/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js b/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js
@@ -29,7 +29,7 @@ describe('HTML without JavaScript gatherer', () => {
       disableJavaScript: true,
       driver: {
         evaluateAsync() {
-          return Promise.resolve('Hello!');
+          return Promise.resolve({bodyText: 'Hello!'});
         },
       },
     };
@@ -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});
     });
   });
 

diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts
@@ -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. */