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

Fix REST API support for plain permalinks. #457

Merged
merged 1 commit into from Aug 8, 2022
Merged

Conversation

peterwilsoncc
Copy link
Contributor

@peterwilsoncc peterwilsoncc commented Jul 28, 2022

Summary

Ensure the WebP fallback JavaScript works with sites using plain permalinks.

Fixes #456.

Relevant technical choices

I decided to copy the approach used in the @wordpress/apiFetch rootUrl middleware that handles this exact issue. It seems to work.

Ensures that the querystring does not have a double ? if the site is configured using plain permalinks.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@peterwilsoncc peterwilsoncc added [Type] Bug An existing feature is broken [Focus] Images Issues related to the Images focus area no milestone PRs that do not have a defined milestone for release [Module] WebP Support Issues for the WebP Support module labels Jul 28, 2022
@bethanylang bethanylang removed the no milestone PRs that do not have a defined milestone for release label Jul 28, 2022
@bethanylang bethanylang added this to Backlog in [Focus] Images via automation Jul 28, 2022
@bethanylang bethanylang moved this from Backlog to Review in [Focus] Images Jul 28, 2022
@mukeshpanchal27 mukeshpanchal27 added this to the 1.4.0 milestone Jul 28, 2022
Copy link
Contributor

@jjgrainger jjgrainger left a comment

Choose a reason for hiding this comment

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

Thanks @peterwilsoncc looks good

Copy link
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@peterwilsoncc Great catch, thanks!

@felixarntz felixarntz merged commit f828649 into trunk Aug 8, 2022
@felixarntz felixarntz deleted the fix/456-rest-route branch August 8, 2022 14:19
[Focus] Images automation moved this from Review to Done Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Images Issues related to the Images focus area [Module] WebP Support Issues for the WebP Support module [Type] Bug An existing feature is broken
Projects
Development

Successfully merging this pull request may close these issues.

WebP Module fallback.js does not support REST API plain permalinks.
6 participants