Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#5368 closed defect (bug) (fixed)

bp_message_thread_excerpt doesn't strip tags

Reported by: henry.wright Owned by: djpaul
Milestone: 2.1 Priority: normal
Severity: normal Version: 1.9.1
Component: Core Keywords: good-first-bug has-patch
Cc:

Description

In bp-messages-template.php, strip_tags() is performed on the message excerpt before the excerpt is retuned. However, tags don't seem to be stripped.

Steps to reproduce:

  1. Send somebody a private message which has a link in the first sentence. Suggested message would be something like: http://buddypress.org/ is cool
  1. Log in as the recipient of the message and navigate to example.com/members/username/messages to view the new message in the inbox list.
  1. Click on the message excerpt (not on the message subject). You'll be taken to http://buddypress.org/

This indicates the <a href="... tag wasn't stripped properly.

Attachments (1)

make-clickable-5368.diff (605 bytes) - added by tw2113 5 years ago.
Remove the add_filter line

Download all attachments as: .zip

Change History (13)

#1 in reply to: ↑ description @henry.wright
6 years ago


This indicates the <a href="... tag wasn't stripped properly.

Actually, perhaps it indicates the a href is being inserted into the excerpt after tags have been stripped? Either way, should the excerpt have tags in it?

#2 @boonebgorges
6 years ago

It looks like we're running bp_get_message_thread_excerpt through make_clickable(). (bp-messages-filters.php)

Does it cause problems for the excerpt to have tags in it?

#3 @henry.wright
6 years ago

Ah right! make_clickable explains it.

Does it cause problems for the excerpt to have tags in it?

No, not at all technically. Although usability wise it might do. For example, when a user is on the inbox screen viewing the list of inbox messages, clicking on the excerpt usually takes them to the message thread conversation page where they can read the full message content. If the excerpt part of the full message content happens to have a link in it, the user could end up on some external website after clicking the excerpt link (which 99% of the time takes them to the full message content). In my opinion, using make_clickable() on the message content is great but not appropriate on the message excerpt.

Last edited 6 years ago by henry.wright (previous) (diff)

#4 @DJPaul
6 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 2.1

I agree with Henry's suggested fix; let's get this done in 2.1.

#5 @DJPaul
5 years ago

To clarify what we think should be done here, it might be as easy as removing this line in bp-messages-filters.php:

add_filter( 'bp_get_message_thread_excerpt', 'make_clickable', 9 );

The not-very-complicated part is just testing that this change addressed the behaviour that henry.wright's nicely described in his last post above.

#6 @tw2113
5 years ago

  • Keywords has-patch added; needs-patch removed

Here's a quick patch for this.

@tw2113
5 years ago

Remove the add_filter line

#7 @DJPaul
5 years ago

Thanks for the patch, tw2113. I just want to check you tested this, and that it fixed/worked as expected?

#8 @tw2113
5 years ago

I didn't get a chance to test it as I didn't really have the dummy data at the time to help. I can look into getting my local to that point if you'd prefer before actually pushing it up. I'm not super familiar with the message thread area of BP, but there's always chance to learn and explore

Last edited 5 years ago by tw2113 (previous) (diff)

#9 @DJPaul
5 years ago

I'll try to test this tomorrow

#10 @djpaul
5 years ago

  • Owner set to djpaul
  • Resolution set to fixed
  • Status changed from new to closed

In 8921:

Messages: don't make links in message excerpts clickable.

When a user is on the inbox screen viewing the list of inbox messages, clicking on the excerpt usually takes them to the message thread conversation page where they can read the full message content. If the excerpt part of the full message content happens to have a link in it, the user could end up on some external website after clicking the excerpt link (which 99% of the time takes them to the full message content).

This change removes make_clickable from operating on the message excerpt, fixing this problem.

Fixes #5368, props tw2113

#11 @DJPaul
5 years ago

henry.wright - I took most of one of your comments to use as part of the commit message. I just wanted to say thanks for describing the situation so nicely. :)

#12 @henry.wright
5 years ago

Hi DJPaul

Glad to have been of some help. Sorry I didn't get the chance to help test

Note: See TracTickets for help on using tickets.