Ending double quote of activity note get mixed with anchor link in Members view

[nirgalo]

(original issue discussion here: http://buddypress.org/support/topic/bug-with-links-posts-in-activity-page/ )

1. go to the Activity page, post a new note such as the following:

here is the link : ​http://google.com

2. go to the Members page, look at the note you just posted : it appears in between double quotes
3. from this Members page, click on the link inside the note

Expected result: new page opens with the link

Actual result: new page opens but the link doesn’t load, because the ending double quote is part of the link (​http://google.com“)

Analysis: the issue seems to originate from this code in bp_get_member_latest_update() (bp-members-template.php):

$update_content = apply_filters( 'bp_get_activity_latest_update_excerpt', sprintf( _x( '- "%s "', 'member latest update in member directory', 'buddypress' ), trim( strip_tags( bp_create_excerpt( $update['content'], $length ) ) ) ) );

When "" and %s" are joined in the form ""%s", the straight double quotes get transformed into fancy double quotes (not sure which area of code is responsible for this transformation), and the closing double quote is being appended to the link itself. Adding a space in between """ and "%s" in order to get "" %s" fixes the problem, quotes are not transformed, and link is proper (but the visual aspect is not fancy).

[nirgalo]

Issue can be found in BP 2.0.1.

[boonebgorges]

I can't reproduce this on my installations.

This problem has come up in the past. See #2443. In r4200, we switched make_clickable() filters to run before wptexturize() (the latter function being the one responsible for the curly quotes), so that we would avoid this specific issue.

The fact that you're still having the problem makes me think that there may be something on your installation that is causing wptexturize() to be run earlier in the load order. nirgalo, can you say more about your setup? What theme are you using? What plugins/customizations? Have you reproduced this bug on a stock installation of BP?

[nirgalo]

I am using this theme: ​http://wordpress.org/themes/ward
In terms of plugins I have : bbPress, Bowe Codes, Insert PHP, s2member, WP-Polls.

I only tired in this environment.

In order to handle things properly independently of the filter order, is is possible to check whether smart quotes have been applied already and if such make special processing?

[boonebgorges]

Thanks for the details. I'll have a look at the theme, though I'd be surprised if that (or the plugins you've listed) are the source of this issue.

In order to handle things properly independently of the filter order, is is possible to check whether smart quotes have been applied already and if such make special processing?

Not really. If the problem is what I suspect, it's that someone is running your content through wptexturize() before BP has a chance to do so. There's no reliable way for BP to tell whether that's already happened for a given string.

If anyone else has thoughts about this, please chime in.

[nirgalo]

I enabled the "Query Monitor BuddyPress conditionals" plugin to check what is being called. I did not see calls to wptexturize (but probably those don't show up). Will it help if I put there the Query Monitor output?

[boonebgorges]

Thanks, nirgalo. Use of wptexturize() wouldn't show up in a query trace, because it doesn't require any calls to the database. One simple test would be to log a backtrace whenever wptexturize() is called, but I'm a bit wary of suggesting that you do this because I don't know about your setup (in particular, you may not want to do this on a busy production site). In brief, you can do this by putting the following into the wptexturize() function:

error_log( print_r( debug_backtrace(), true ) );

Though, to be honest, this will only be of limited value if wptexturize() is being hooked rather than called directly (in the former case, it won't show up directly in the stack trace).

Another thought is simply to search your codebase for the function name:

$ grep -nR 'wptexturize' wp-content/plugins
$ grep -nR 'wptexturize' wp-content/themes

[nirgalo]

the only references I have on wptexturize are inside buddypress and bbpress plugins. Nothing in the theme or other plugins (except Hello Dolly which is not enabled).

[boonebgorges]

Hm. I still can't recreate. nirgalo, is your site in English? I wonder if this could have to do with language packs.

[nirgalo]

The site is in French.

[boonebgorges]

OK, thanks. And the quotes that you are seeing: are they English-style curly quotes, or are they French-style guillemets?

[nirgalo]

they seem to be french guillemets, see attached.

[nirgalo]

ending double quotes as part of link

[boonebgorges]

Aha. I've reproduce the problem when using fr_FR. I'll look into it a bit more.

See also #4265

[nirgalo]

Awesome! To fix this locally I just removed the quotes from the string... not elegant but better than adding up spaces.

[boonebgorges]

OK. After some digging, it looks like the problem is as follows. make_clickable is applied to the content sent through the `bp_get_activity_latest_update_excerpt' filter. But the value that we're currently passing to this filter is the excerpt sprintf()ed into the wrapper. So, for the following update

Check out this site: http://buddypress.org

the string that is being made clickable is

- "Check out this site: http://buddypress.org "

The extra space between .org and " in BP's source code tells make_clickable() not to include " in the URL. But in the French translation of the wrapper string:

msgid "- "%s ""
msgstr "- «%s»"

the extra space is not present. So make_clickable() thinks that the trailing guillemet is part of the URL.

I wonder if this extra space is intentional. Looking back on the original ticket #3505, I see that it was added in between patch 01 and 02. r-a-y, maybe you can shed some light on this :)

In any case, it's a bug in BP that we're doing it this way. For 100% backward compatibility, my initial thought was that we should create a new filter for the excerpt alone, and preserve the existing _excerpt filter on the sprintf()ed string. But I now see that we are using the same filter name in the activity component on a standalone excerpt: ​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-activity/bp-activity-template.php#L2527 So it seems like we should probably not add a second filter, and just fix the placement of the first one so that it's filtering only the excerpt. (Fixing two bugs with one stone!)

In my fix, I am also going to remove the extraneous space from the wrapper string ('- "%s"' instead of '"%s "'). nirgalo, this will break your existing translation, so you may want to update your .po/.mo file manually if you want to be able to test the changes before BP 2.1 comes out in a few months.

[boonebgorges]

In 8400:

Standardize placement of 'bp_get_activity_latest_update_excerpt' filter

When latest activity update excerpts are displayed in the Members directory,
they are displayed like this:

• "Foo"

instead of just

Foo

The wrapped string is then sent through the 'bp_get_activity_latest_update_excerpt'
filter. However, this creates inconsistencies with other uses of the
filter, which only filter the excerpt itself - not the dash and quotes. This
changeset fixes the inconsistency.

In addition, certain translations of the wrapper string were removing an
erroneous space character from the original string, which had been preventing
the make_clickable() callback from appending the trailing quote to URLs that
appeared at the end of the activity excerpt. In these translations (such as
fr_FR), the result was that the improper filter placement described above was
causing trailing quotation marks to be included as part of URLs when they
appeared at the end of an update string (from the point of view of make_clickable()).
This changeset also removes this extra space, since the bug will no longer
manifest itself in any language.

Fixes #5626