Skip to:
Content

Opened 2 years ago

Last modified 20 months ago

#7084 assigned defect (bug)

@mentions break instagram oembeds if same username exists on site as instagram

Reported by: 3pointross Owned by: johnjamesjacoby
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version: 1.2
Component: Activity Keywords: reporter-feedback
Cc:

Description

Instagram embeds include @<username> in their markup, which is parsed by BuddyPress. If the same username exists on the site, it's converted to a profile link which disrupts the final loading of the embed.

Attachments (1)

7084.01.patch (2.4 KB) - added by r-a-y 20 months ago.

Download all attachments as: .zip

Change History (19)

#1 @DJPaul
2 years ago

  • Milestone changed from Awaiting Review to Future Release

#2 @johnjamesjacoby
2 years ago

  • Keywords needs-unit-tests needs-patch early added
  • Milestone changed from Future Release to 2.6
  • Owner set to johnjamesjacoby
  • Priority changed from normal to high
  • Severity changed from normal to major
  • Status changed from new to assigned
  • Version set to 1.2

Hey @3pointross, thanks for the report.

You're right, and this is something we've identified independently, too. The fix is less simple than we initially imagined, and the implications are also a bit more reaching, so we held off on publishing anything until we had more to go on.

I'm prioritizing this ticket in our queue, and I'll reply back with a patch and progress ASAP.

#3 follow-up: @3pointross
2 years ago

Thanks for the response @johnjamesjacoby, if there is anything I can do to assist let me know, I'd be happy to help.

#4 in reply to: ↑ 3 @johnjamesjacoby
2 years ago

Replying to 3pointross:

Thanks for the response @johnjamesjacoby, if there is anything I can do to assist let me know, I'd be happy to help.

Will do. If you're comfortable testing a patch when it's ready, that will be the best place to start confirming that we've effectively fixed the issue that you've identified. If you're able to help with improving the code in that patch, even better, but don't feel obligated; we're just happy to have the report here. :)

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


2 years ago

#6 @dcavins
2 years ago

  • Milestone changed from 2.6 to 2.5.4

#7 @DJPaul
2 years ago

  • Milestone changed from 2.5.4 to 2.6.1

#8 @dcavins
2 years ago

  • Milestone changed from 2.6.1 to 2.6.2

@DJPaul: I'm punting this a little. I'll leave it to you to punt to 2.7 if appropriate.

This ticket was mentioned in Slack in #buddypress by mercime. View the logs.


22 months ago

This ticket was mentioned in Slack in #buddypress by mercime. View the logs.


22 months ago

#11 @dcavins
22 months ago

  • Milestone changed from 2.6.2 to 2.7

#12 @websupporter
22 months ago

In order to understand the BP system a little better, I thought it's a good idea to learn its bugs, so I dived into this ticket, to see whats going on.
As mentioned by others its a bit wired and it looks, some problems are interfering with each other.

So, I searched some Instagram Messages, created users which where also mentioned in the instagram message and tried it myself. Hmm... where to find now messages! Ha Ivanka Trump should mention her @realdonaldtrump quite often (actually nope and sorry for this example :/).

So what I've found. I am not quite sure:

Ivanka has a problem. Some of her Instagram messages do not show properly in her BuddyPress community.

The ajax request

I've created the user @trumpturnberryscotland and embeded https://www.instagram.com/p/BJQHeeygGXc/
It doesn't show with the ajax request, but it shows for me with F5! (Actually I do not have a problem when @trumpturnberryscotland is mentioned in the message?)

Matt West (first, who showed up in my timeline XD ) has no problem, when he wants to embed his Tweets:
https://twitter.com/MattAntWest/status/766884281100619776
It shows also with the ajax request and he doesn't need to reload. He is happy.

Whats the difference between Twitter and Instagram? Instagram is embeding its scripts with asyn and defer, Twitter uses only asyn.

This Boolean attribute (defer) is set to indicate to a browser that the script is meant to be executed after the document has been parsed, but before firing DOMContentLoaded

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script

Well, I think here we can't help Ivanka? Instagram might.

The make clickable problem

But Ivanka has another, more persistent problem too:
https://www.instagram.com/p/BJOQHUgAYLP/

The oEmbed HTML does not link "http://liketk.it/2p0HO" (while Matt West is happy using Twitter. Twitter wraps links in <a>-Tags).

In the filter bp_get_activity_content_body we transform a URL which is embeddable into the embed with the priority of 8 and do run make_clickable() with the priority of 9. So make_clickable() sees the URL and makes it clickable (You're welcome Instagram). Has anyone tried to read the instagram embed script. \o/

So, I didn't completely, but what I think are the basics:

<?php
if(xa.tagName==='BLOCKQUOTE'){
        var ya=oa(xa);
        if(ya)
                pa(xa,ya);
}

function oa(ua){
        var va=ua.getElementsByTagName('a'),wa=va.length;return wa?na(va[wa-1]):null;
}

function pa(ua,va){var wa=ka++,xa=r+wa;if(!ua.id)ua.id=da+wa;var ya=va.replace(w,'/$1');ya+='embed/';if(ua.hasAttribute(q))ya+='captioned/';if(ua.hasAttribute(ga)){var za=parseInt(ua.getAttribute(ga),10);if(m(za))ya+='?v='+za;}ya=ya.replace(u,v);var ab;ab=document.createElement('iframe'); //and so on

The basic HTML where it should be applied to:
<blockquote><a href="link-to-instagram-message">text http://liketk.it/2p0HO by ivanka and so on</a></blockquote>

Without further testing, I think the function oa() returns the wrong <a> attribute out of wich pa() can not create the iframe properly. Why? Because we make the URL 'http://liketk.it/2p0HO' clickable:
<blockquote><a href="link-to-instagram-message">text <a href="http://liketk.it/2p0HO" rel="nofollow">http://liketk.it/2p0HO</a> by ivanka and so on</a></blockquote>

In my tests, if I remove make_clickable from the filter it works. So I thought, maybe I just give it the priority of 7, but in this case our embed-filter does not work anymore, because the URL needs to be singled out. We just have a URL to the embed. Ha Ivanka! Sorry Matt.

But what if...

We reduce the make_clickable priority 7 and

We extend BP_Embed with

<?php
        public function autoembed( $content ) {
                
                // Replace line breaks from all HTML elements with placeholders.
                $content = wp_replace_in_html_tags( $content, array( "\n" => '<!-- wp-line-break -->' ) );

                if ( preg_match( '#(^|\s|>)https?://#i', $content ) ) {
                        // Find URLs on their own line.
                        $content = preg_replace_callback( '|^(\s*)(https?://[^\s<>"]+)(\s*)$|im', array( $this, 'autoembed_callback' ), $content );
                        // Find URLs in their own paragraph.
                        $content = preg_replace_callback( '|(<p(?: [^>]*)?>\s*)(https?://[^\s<>"]+)(\s*<\/p>)|i', array( $this, 'autoembed_callback' ), $content );
                        // Find URLs in their own paragraph.
                        $content = preg_replace_callback( '|^(<a(?: [^>]*)?>\s*)(https?://[^\s<>"]+)(\s*<\/a>)|i', array( $this, 'autoembed_callback' ), $content );
                }

                // Put the line breaks back.
                return str_replace( '<!-- wp-line-break -->', "\n", $content );
        }

see especialle the regex ^(<a(?: [^>]*)?>\s*)(https?://[^\s<>"]+)(\s*<\/a>)|i

In this case we say embeds can also be placed in an <a>-tag. Probably it needs some more thought and work on the regex and maybe it has some hidden problems, I do not see, but Ivanka and Matt seem to be happy on the first glance.

Note:
I am also not sure, if this corresponds with the bug reported in this ticket or if I found a new problem or if I simply smashed my install so hard while playing that this is not a problem at all in a clean installation :D I just hope it helps.

This ticket was mentioned in Slack in #buddypress by websupporter. View the logs.


21 months ago

#14 @r-a-y
20 months ago

  • Keywords has-patch added; needs-unit-tests needs-patch early removed

Thanks for the rundown, @websupporter.

I think the proper way to fix this is to replace instances of @ with &#64; when grabbing the oEmbed HTML. Since our at-mention filter looks explicitly for the @ sign, this is a decent workaround. We should do this only for at-mentions that are not already wrapped in links.

01.patch includes unit tests and should fix this issue.

Last edited 20 months ago by r-a-y (previous) (diff)

@r-a-y
20 months ago

This ticket was mentioned in Slack in #buddypress by mercime. View the logs.


20 months ago

#16 @r-a-y
20 months ago

  • Keywords reporter-feedback added; has-patch removed
  • Milestone changed from 2.7 to Awaiting Review

Actually, this needs more testing. I cannot duplicate the problem.

@websupporter even mentions this in comment:12:

I've created the user @trumpturnberryscotland and embeded https://www.instagram.com/p/BJQHeeygGXc/
It doesn't show with the ajax request, but it shows for me with F5! (Actually I do not have a problem when @trumpturnberryscotland is mentioned in the message?) (emphasis added)

I can confirm websupporter's example.

@3pointross - Can you let us know what Instagram links you are trying and some steps to duplicate your problem?

#17 @r-a-y
20 months ago

  • Priority changed from high to normal
  • Severity changed from major to normal

#18 @DJPaul
20 months ago

  • Milestone changed from Awaiting Review to Future Release
Note: See TracTickets for help on using tickets.