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

Adding a new class for properly handling Instagram oembeds in previews #263

Merged
merged 2 commits into from Sep 13, 2016

Conversation

david-binda
Copy link
Contributor

Since the WordPress is calling the make_clickable function in terms of the comment_text filter, Instagram oEmbedes containing a plaintext link are not being properly initialised as Instagram's JS expects only a single a element inside the blockquote.

This commit is programmatically removing the script element using DomDocument and is appending a custom one, which 1) load the original script only if it has not been loaded before 2) initialise the Instagram oEmbed manually (as a side effect this fixes the issue with Instagram oEmbeds not properly working when clicking the preview button multiple times).

Since the WordPress is calling the `make_clickable` function in terms of the `comment_text` filter, Instagram oEmbedes containing a plaintext link are not being properly initialised as Instagram's JS expects only a single `a` element inside the `blockquote`.

This commit is programmatically removing the script element using DomDocument and is appending a custom one, which 1) load the original script only if it has not been loaded before 2) initialise the Instagram oEmbed manually (as a side effect this fixes the issue with Instagram oEmbeds not properly working when clicking the preview button multiple times).
@philipjohn
Copy link
Contributor

Thanks @david-binda !

This does seem like a well targeted approach. I'm wondering if the JS we're adding there is effectively a copy of what Instagram provides. If so, do we open ourselves up to bugs if they update their JS and we're swapping that out with an older version?

@david-binda
Copy link
Contributor Author

I'm wondering if the JS we're adding there is effectively a copy of what Instagram provides.

It is not a copy of what Instagram provides. We are still loading the one Instagram provides, just not via standard <script> element, but conditionally (only when needed) via jQuery's $.getScript. That's the first part of the custom script.

The second part of the script is removing <a> elements added by make_clickable filter inside the Instagram's blockquote element.

The third and final part is calling well documented instgrm.Embeds.process() funciton for initialising the oEmbeds in case someone is loading the script on their own (like we do in this commit).

So in case Instagram updates the script, we still will be fine, since we're loading it. The problem might come up in case they changed the location. But since the location is documented and the documentation provides information on how to call the function to initialise the oEmbed, I bet that they would notify developers about changes in the location or in the functionality before simply changing that.

This commit removes the DOM Document parsing of the oEmbed response in favour of taking advantage of `oembed_fetch_url` filter for requesting the oEmbed without the script
@philipjohn philipjohn added this to the 1.5.2 milestone Sep 13, 2016
@philipjohn philipjohn merged commit d369b7e into master Sep 13, 2016
@philipjohn philipjohn deleted the fix-instagram-oembed-preview branch September 13, 2016 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants