Embed: Avoid retrying valid URLs #60655
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mamaduka
added
[Type] Bug
|
Size Change: +33 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
Mamaduka
commented
Apr 11, 2024
Comment on lines
+197
to
+198
| 'https://twitter.com/notnownikki/': | ||
| MOCK_BAD_EMBED_PROVIDER_RESPONSE, |
There was a problem hiding this comment.
The MOCK_CANT_EMBED_RESPONSE returns a fallback preview.html, causing the side-effect to be skipped.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
t-hamano
approved these changes
Apr 12, 2024
There was a problem hiding this comment.
LGTM! Thank you for the quick fix 🙇
URLs that support trailing slashes
https://developer.wordpress.org/block-editor/reference-guides/block-api/block-attributes/
The request is executed only once and the trailing slash is not removed.
URLs that do not support trailing slashes
https://twitter.com/WordPress/
Retry with URL without trailing slash.
P.S. An issue has been reported on the Japanese forum related to this issue. This PR will fix that issue as well.
It was reported as below.
Hi.
I recently updated to Wordpress 6.5.
However, after updating, a problem has occurred where all trailing slashes in internal links (set in embedding blocks) are deleted.
https://example.com/example/ → https://example.com/example
Also, when I discovered the above, I entered the trailing slash and updated the article, but when I reopened the editing screen, the trailing slash had been deleted again.
As a result, a large number of URLs without trailing slashes have accumulated in the redirect target of the plugin "Local Broken Link Checker" and in the Google Search Console "Page has a redirect".
What can I do to resolve the phenomenon where trailing slashes in internal URLs set in embedded blocks are deleted?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Regression: #58007 (comment).
PR fixes the Embed block side-effect for retrying failing URL embeds without trailing slash. The fix prevents unnecessary retries for providers that support trailing slashes.
Why?
The logic had a race condition, causing false runs in the side-effect logic.
When the URL is newly submitted and the
getEmbedPreviewstarts the resolution:fetchingflag isfalse, waiting for meta selectors to return value.cannotEmbedistrue. The selector hasn't returned any info related to the previews.previewisundefined. Same as above, the selector hasn't returned the value yet.When all these conditions are met while the selector hasn't finished the resolution, the side effect logic runs and unnecessarily retries the URL.
How?
Check if
getEmbedPreviewhas resolved the initial URL before attempting the retry.Testing Instructions
https://developer.wordpress.org/block-editor/reference-guides/block-api/block-attributes/.https://twitter.com/WordPress/.Testing Instructions for Keyboard
Same.