new_audit: bring back redirects-http with passive https check #13548
Conversation
brendankenny
requested review from
connorjclark
and removed request for
a team
connorjclark
left a comment
connorjclark
left a comment
There was a problem hiding this comment.
Sorry for late approval.
| lhr: { | ||
| requestedUrl: 'https://jakearchibald.github.io/svgomg/', | ||
| // Intentionally start out on http to test the redirect. | ||
| requestedUrl: 'http://jakearchibald.github.io/svgomg/', |
There was a problem hiding this comment.
We have a few redirects smoke tests, should it go there instead?
Otherwise, please add an exclusion to these expectations so we can still run these smokes in devtools runner.
connorjclark
force-pushed
the
resurrect-redirect
branch
from
d157320 to
0efec59
Compare
|
@brendankenny branch updated |
connorjclark
force-pushed
the
resurrect-redirect
branch
from
0efec59 to
a598869
Compare
connorjclark
force-pushed
the
resurrect-redirect
branch
from
a598869 to
7759b98
Compare
| Object.assign(/IndexedDB/, {_runner: 'devtools'}), | ||
| // DevTools can't simply test a redirect, because the browser will have already navigated before the Lighthouse panel can begin. | ||
| Object.assign(/redirected/, {_excludeRunner: 'devtools'}), | ||
| ], |
connorjclark
force-pushed
the
resurrect-redirect
branch
from
7759b98 to
c848f22
Compare
| Object.assign(/IndexedDB/, {_runner: 'devtools'}), | ||
| // DevTools can't simply test a redirect, because the browser will have already navigated before the Lighthouse panel can begin. | ||
| Object.assign(/redirected/, {_excludeRunner: 'devtools'}), | ||
| ], |
|
It's been two years but it seems like this would still be useful. |
|
Doesn't appear to be any downsides to this. I'll update for 12.0. |
|
Aaaaactually I'm pretty sure So maybe we just close this? |
Conceptually they're different checks. The context of this PR is that there is a difference, and it would be little effort (and none of the complexity that prompted deleting the audit in the first place would be required) to keep the audit, so why not keep it. On the other hand, yes, a lot of overlap, and the path to being able to pass both if you start a LH run on an insecure URL is very narrow (just preloaded/cached HSTS these days? Any other approah?). In #3417 there was a proposal to relax My quick opinion two years later: I think it's still good to warn users when they're not using HTTPS or not upgrading, both of which #15907 communicates (albeit less clearly without a dedicated description). I wish we could do more with giving advice about the connection upgrade etc, but that's not the direction best practices is going, so it's fine. |
4 participants
hot-take/needs-discussion PR: Bring back the
redirects-httpaudit, but without the extra pass.requestedUrlis onhttpandfinalUrlis onhttps, passfinalUrlisn't onhttps, failrequestedUrlis on any other scheme, notApplicableredirects-httpwas removed in #12643 due to a history of debugger protocol issues, the massive slowness of an extra pass, and the fact that Chrome switched to https by default if a user doesn't supply a scheme.However there are still sites out there that should be redirecting to https and Lighthouse should continue to nudge the remaining 10-20% of sites people spend time on to do so if it's feasible.
We already have this audit in git history ready to go with just a few changes, and switching to only alerting when someone is purposefully testing their http site should take user complaints down to zero (for instance, the old check required the http version of a site to respond, if only to redirect), which also allows making the audit essentially zero-overhead by passively observing what the
URLartifact is collecting anyways.Thoughts? :)