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

False positive on noscript tag #5910

Closed
adaschevici opened this issue Aug 25, 2018 · 18 comments
Closed

False positive on noscript tag #5910

adaschevici opened this issue Aug 25, 2018 · 18 comments

Comments

@adaschevici
Copy link
Contributor

Summary

I created a new app using preact-cli:

preact create default <appname>
cd <appname>
npm run serve

screen shot 2018-08-25 at 10 33 02 am

I added a noscript tag in the main component and it is shown when i disable JS however the score on lighthouse is still at 82 and it says that i should add a noscript tag.

    return (
      <div id="app">
        <noscript>
          <div style="position: fixed; top: 0px; left: 0px; z-index: 3000; 
                      height: 100%; width: 100%; background-color: #FFFFFF">
            <p style="margin-left: 10px">JavaScript is not enabled.</p>
          </div>
        </noscript>
				<Header />
				<Router onChange={this.handleRoute}>
					<Home path="/" />
					<Profile path="/profile/" user="me" />
					<Profile path="/profile/:user" />
				</Router>
			</div>
    );
  }
@fenilkanjani
Copy link

+1
Content of the noscript tag is visible on disabling javascript. But the audit still fails the test.

@justinribeiro
Copy link
Contributor

Cursory glance, this seems not specific to the <noscript>, but rather to the way it's served with npm run serve locally. If the default gen'ed app is served another way, both the noscript and the SSR'd portions of the preact app successfully pass said audit.

@patrickhulce
Copy link
Collaborator

+1 to @justinribeiro 's findings. I'm not able to reproduce this with the provided steps. On a new preact project with your snippet and yarn build with a static server, the audit passes for me on latest Lighthouse.

@adaschevici
Copy link
Contributor Author

ok @justinribeiro , it seems to be a problem in the way the app is being served:
When running it in dev mode without building the noscript warning is addressed when I add a <noscript>test</noscript>
My current npm scripts section looks like this its from the defaults:

    "start": "if-env NODE_ENV=production && npm run -s serve || npm run -s dev",
    "build": "preact build",
    "serve": "preact build && preact serve",
    "dev": "preact watch",
    "lint": "eslint src",
    "test": "jest ./tests"

If I get correctly what you're saying, if the app is served using something other than npm or preact serve then the audit should pick it up?
Would that mean that preact serve is behaving in an unexpected manner when run with lighthouse audits?
From what you're saying it seems that the build step causes the audit to fail 🤔but only when built with npm?
The versions I am running are latest nvm using 10.9.0 and latest 6.4.0 npm.

@patrickhulce
Copy link
Collaborator

When you look at the contents of your build directory, does the index.html file have the noscript tag?

@adaschevici
Copy link
Contributor Author

@patrickhulce yes, it does

@justinribeiro
Copy link
Contributor

Would that mean that preact serve is behaving in an unexpected manner when run with lighthouse audits?

As I understand it, preact serve uses simplehttp2server. Testing using just simplehttp2server on some random html files (preact gen'ed or not) that contain <noscript> or other just non-JS rendered HTML, I'm able to duplicate the problem.

I'll have to dive a little further into simplehttp2server and see if that's where the particular issue lies.

@fchristant
Copy link

I hope you don't mind me using this thread to report another false positive of this same test.

I have a classic LAMP application, so server-rendered. The entire page is HTML + CSS, with only JS for some added interactivity. When you disable JS entirely in the browser, the page fully renders just fine.

Still it fails this test. I'm not using noscript but I shouldn't have to because JS isn't used for rendering at all. So when reading this description:

"Lighthouse disables JavaScript on the page and then inspects the page's HTML. If the HTML is empty then the audit fails. If the HTML is not empty then the audit passes."

Somehow it fails to do this.

@patrickhulce
Copy link
Collaborator

@fchristant do you have a public URL for us?

@fchristant
Copy link

@patrickhulce Sure, but this is a VM running on my home desktop so it wont be up for long. I'll try to keep it on until you had a look:
https://www.eon.earth/silva

@adaschevici
Copy link
Contributor Author

@justinribeiro so you think it may be something in simplehttp2server that is causing the issue?
Sounds like a good avenue of investigation.
Having had a quick look at the npm package it seems to be a thin wrapper around a c binary from google chrome https://github.com/1000ch/simplehttp2server/blob/master/lib/index.js#L7

@patrickhulce
Copy link
Collaborator

Thanks for sharing @fchristant I'm able to reproduce the issue there, that's really strange. No immediate ideas on what's causing it... 🤔

@justinribeiro
Copy link
Contributor

Looking at the output from both @fchristant url and preact serve / simplehttp2server, it appears that HTTP redirect pass also fails (see overly verbose screenshot below). In this case, I suspect this is because HTMLWithoutJavaScript and HTTPRedirect run in the same pass, so when the HTTP redirect request fails, the the second audit fails as well.

In the case of preact serve / simplehttp2server, I can see this locally on the CLI as the error http: TLS handshake error from ..., which means it's not returning the document to LH on the redirect, at which point the HTMLWithoutJavaScript also fails. This behavior is mentioned to some extent in #2784 as server misconfiguration.

image

@adaschevici
Copy link
Contributor Author

@justinribeiro does that mean that if I configure the https on the preact app so that it manages to do the TLS handshake then the noscript tag would be picked up and the LH score would be reflected in the app?

@justinribeiro
Copy link
Contributor

It's a known issue with simplehttp2server that the connection is reset for the http > https redirect (there's even an older comment noting this causes issues for LH) GoogleChromeLabs/simplehttp2server#33

@adaschevici
Copy link
Contributor Author

awesome @justinribeiro thanks for all the help.
you can close this issue as i think it's fairly well explained.
maybe keep a reference for known issues unless you think it is too much of a corner case

@patrickhulce
Copy link
Collaborator

Oooooh of course, I forgot about this thank you very much for tracking it down @justinribeiro! #1217 captures this issue as well. I'll go ahead and close, but I'm open to a PR adding this note to the documentation :)

@fchristant
Copy link

Just want to confirm that after fixing my http->https redirect on the example site I gave, the audit passes successfully now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants