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

Implement basic dev server HTTPS certificate error detection #12

Merged
merged 1 commit into from
Aug 3, 2019

Conversation

kadamwhite
Copy link
Collaborator

This needs a spot of refactoring -- a new namespace is probably in order, at minimum -- but I've implemented an idea I discussed this morning with @shadyvb to make it more obvious when we've forgotten to accept the dev server cert. At present this only handles the Block Editor admin-side case, as we have a more reliably available notices framework in that context.

image
Show a notice to the user if we attempt to load a script from an HTTPS localhost address and that script encounters a loading error. This usually indicates that we forgot to accept the Webpack DevServer SSL cert.

![image](https://user-images.githubusercontent.com/442115/62392867-31321680-b52e-11e9-88f9-690430ed0d77.png)
Show a notice to the user if we attempt to load a script from an HTTPS
localhost address and that script encounters a loading error. This usually
indicates that we forgot to accept the Webpack DevServer SSL cert.
@shadyvb
Copy link

shadyvb commented Aug 3, 2019

Thanks for adding this @kadamwhite ! Saves a few minutes of confusion, and couple of clicks to approve the certificate as well! 🎉

@johnbillion
Copy link
Member

Neat! That will save some head scratching.

You can also do this globally without the onerror attribute, I've got a branch of Query Monitor for exactly this. I'll finish it up as soon as I can.

@kadamwhite
Copy link
Collaborator Author

@johnbillion would love to compare notes on that when you get a chance, I try to global approach and it did not work as expected.
@shadyvb thanks for the review!
@ both y’all, there’s definitely room for improvement but for now I’ll land this and open a new PR for refactoring.

@kadamwhite kadamwhite merged commit 4f961f5 into master Aug 3, 2019
@shadyvb
Copy link

shadyvb commented Aug 5, 2019

FYI : Chassis/chassis_openssl#4 to fix the underlying issue, after which, on Chassis projects, we can reuse the existing SSL cert and forget about this altogether.

@shadyvb shadyvb deleted the warn-on-https-dev-issues branch August 5, 2019 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants