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

Add note for using 302 redirect in HTTP connections for WebP #1334

Merged
merged 1 commit into from
Dec 31, 2020

Conversation

peter-gribanov
Copy link
Contributor

Q A
Branch? 2.0
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

For more details see this discussion: #1319 (comment)

@lsmith77 lsmith77 merged commit 906b712 into liip:master Dec 31, 2020
@peter-gribanov peter-gribanov deleted the webp_http_doc_note branch December 31, 2020 17:04
@@ -264,4 +264,20 @@ If browser supports WebP, the request ``https://localhost/media/cache/resolve/th
will be redirected to ``https://localhost/media/cache/thumbnail_web_path/images/cats.jpeg.webp``
otherwise to ``https://localhost/media/cache/thumbnail_web_path/images/cats.jpeg``

.. note::

Using an unsecured connection (HTTPS) on your site can cause problems with
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsecured connection (HTTPS) this does not seem to be right. Unsecured is not HTTPS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Exactly. It seemed to me that writing unsecured connection (HTTP) would not be clear enough. Therefore, i'm talking about HTTPS, hinting at the security of the connection. Perhaps it would be better to write unsecured connection (non HTTPS).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you send a new PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsmith77 yes of course #1340

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