#57376 closed defect (bug) (fixed)
Fix Mixcloud Embed url
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Embeds | Keywords: | has-patch needs-dev-note |
Focuses: | Cc: |
Description
I work at Mixcloud and we are moving some api's about as part of a large hygiene project, the new api is online and we are opening tickets/patches to move over key partners.
https://www.mixcloud.com/oembed/
to
https://app.mixcloud.com/oembed/
Attachments (3)
Change History (28)
This ticket was mentioned in PR #3799 on WordPress/wordpress-develop by matclayton.
2 years ago
#1
- Keywords has-patch added
#4
@
2 years ago
Thanks for the pull request @matclayton!
As part of the clean up, would it be possible to add the oembed discovery headers to the HTML for mixes?
They're detailed in the Discovery section of the oembed specification.
I was testing doing a curl request to https://www.mixcloud.com/FatboySlim/fatboy-slim-live-from-glastonbury-2022-shangrila-gas-tower/
and was unable to see them. It appears they are added via JavaScript.
#5
@
2 years ago
Thats 100% what we are trying to achieve. Unfortunately its a bit of a mess and a long series of changes. For a broader context we have our own Server side rendering system, which we are repairing, to fix up use cases like this. But to do that we're removing all the apis on that domain to allow us to re-architect fix it cleanly, which is how we got to this PR :)
Suffice to say this PR is one along that journey, ultimately fixing up the discovery headers and other related SSR bugs along the way.
Hopefully we can merge this in without the headers as it'll unblock a lot of other work which will ultimately allow us to fix up discovery and other bugs.
#6
@
2 years ago
- Milestone changed from Awaiting Review to 6.2
Yeah, it should be fine to get in for the next major WordPress release. I've moved it to the milestone.
While you're modifying URLs, something to be aware of is that WordPress caches the oembed endpoint's response when posts are saved. You can see this in the wp_postmeta
table.
This means that if you ever move the www.mixcloud.com/widget/iframe/*
URLs, old WordPress posts will continue to request them so you'll need to include redirects, maintain them or similar.
Y'all are probably across this but I'd sure feel foolish if it became a problem and I didn't mention it while I had the chance ;)
#7
@
2 years ago
Hi @peterwilsoncc,
Thanks for the heads up, we 100% plan to support redirects here, interestingly we ended up doing this patch as the oEmbed jetpack code wouldn't follow redirects for the oEmbed endpoint to the new one. Once we'd fixed that they advised we also come and do the same patch over here, to mitigate any issues with 301's not being followed.
Mat
#8
@
2 years ago
- Keywords reporter-feedback added
- Owner set to audrasjb
- Status changed from new to reviewing
Hello @matclayton and thanks for opening this ticket,
I was going to commit this patch, but… is this change already published/live?
I tested the patch and it looks like it doesn't embed Mixcloud content anymore (see screenshots above this comment).
#9
@
2 years ago
The endpoint is live currently, so I'm not sure what is going on there. you should be able to see the responses on the two versions below and confirm they are identical. Do you have any more exceptions on your end?
Old:
https://www.mixcloud.com/oembed/?url=https://www.mixcloud.com/rez/&format=json
New:
https://app.mixcloud.com/oembed/?url=https://www.mixcloud.com/rez/&format=json
#10
@
2 years ago
- Keywords reporter-feedback removed
Oh, ok. I was probably experiencing a caching issue, sorry for the false alarm.
The patch works fine on my side 👍
@audrasjb commented on PR #3799:
2 years ago
#13
Committed in https://core.trac.wordpress.org/changeset/55068
#15
@
2 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Coming back to this, I believe there may be an issue with the embed. This may be what caused the problems @audrasjb ran into when he tested the change.
When querying https://www.mixcloud.com/oembed?maxwidth=600&maxheight=750&url=https%3A%2F%2Fwww.mixcloud.com%2FTheBlessedMadonna%2Fwe-still-believe-episode-085-new-release-wall-special%2F&dnt=1&format=json
, you get the proper embed response in the body, after being redirected to https://app.mixcloud.com/oembed/?maxwidth=600&maxheight=750&url=https%3A%2F%2Fwww.mixcloud.com%2FTheBlessedMadonna%2Fwe-still-believe-episode-085-new-release-wall-special%2F&dnt=1&format=json
-> This was the query you would have made with the old format (currently in Core).
However, when querying https://app.mixcloud.com/oembed?maxwidth=500&maxheight=750&url=https%3A%2F%2Fwww.mixcloud.com%2FTheBlessedMadonna%2Fwe-still-believe-episode-085-new-release-wall-special%2F&dnt=1&format=json
directly, you get an empty body. Notice the missing trailing /
before the query parameters in the URL that fails.
-> This is the new embed format introduced with this changeset.
Should we update Core to have that trailing slash, or revert that change in Core to use the old format (that still works with the redirect) until Mixcloud can support URLs both with and without trailing slashes?
Thank you!
#16
@
2 years ago
Somewhat related: the embeds seem to work when the Gutenberg plugin is active, because Gutenberg still uses the old format:
https://github.com/WordPress/gutenberg/blob/88335dbf42f456b2c89e3fa12ade3d5170100daa/packages/block-library/src/embed/variations.js#L217
#17
@
2 years ago
@jeherve Thanks for reporting this, I'll get a patch out on www.mixcloud.com to fix this up. They'll be other platforms having the same issue and not just WordPress. Technically the / version is the one we support in the docs, but enough folk will trip up over this, we need to fix it up.
Mat
#18
@
2 years ago
This fix is now out.
See
https://app.mixcloud.com/oembed?maxwidth=500&maxheight=750&url=https%3A%2F%2Fwww.mixcloud.com%2FTheBlessedMadonna%2Fwe-still-believe-episode-085-new-release-wall-special%2F&dnt=1&format=json
Renders as JSON, we've made it work with and without the /
#19
@
2 years ago
Thanks for the prompt fix upstream.
Given the documented endpoint includes a trailing slash, I suggest WordPress uses it per 57376.diff.
#22
follow-up:
↓ 23
@
2 years ago
@milana_cap Sorry I'm not quite sure what you mean by that, what do you need?
I think this could be summed up as:
"Adds a trailing slash to the mixcloud.com oembed endpoint to match the documented version."
As stated above
#23
in reply to:
↑ 22
@
2 years ago
Replying to matclayton:
@milana_cap Sorry I'm not quite sure what you mean by that, what do you need?
I think this could be summed up as:
"Adds a trailing slash to the mixcloud.com oembed endpoint to match the documented version."
As stated above
I'm asking because the ticket was labelled with needs-dev-note
. Considering how small the change is, this could probably go into mixed Dev note or even just in the Field guide. Thank you.
Trac ticket: https://core.trac.wordpress.org/ticket/57376#ticket