Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

#5890 closed defect (bug) (fixed)

Buddypress Activity Link (time ago) Issue

Reported by: Cartographer Owned by: djpaul
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc:

Description

Hi there,

I was told to post a new ticket about an issue.

I have a WordPress/Buddypress installation. I use Groups and each group has its own forum.

Let’s go to the problem now.

A community member creates a topic (under a Group forum) with a big Greek title
(for example: Αυτός είναι ένας εξαιρετικά μεγάλος τίτλος).

Every link pointing to this topic is ok except the time link (freshness column e.g. two minutes ago) in the Sitewide Activity post (which is created automatically). Somehow the title is cut and the link is pointing to a page “Topics not found”

Even the “topic title” link in the same activity post is pointing to the correct page, the problem occurs only in the time link (freshness column e.g. two minutes ago).

Maybe you could try to replicate the issue by adding as an example the title (above) to a new topic and check the activity post (freshness column).

When the title (in Greek) is short e.g. two words, the issue does’t occur.

You can see the conversation in the support forums here: http://buddypress.org/support/topic/buddypress-activity-link-time-ago-issue/#post-197930

Thank you for your time :)

Buddypress: 2.0.3
Wordpress: 4.0
Bbpress: 2.5.4
Theme: Twenty fourteen

Attachments (1)

5890.diff (1.2 KB) - added by tharsheblows 6 years ago.
change primary_link and bump version

Download all attachments as: .zip

Change History (19)

#1 @boonebgorges
6 years ago

  • Keywords reporter-feedback added

Thanks for the report, Cartographer.

It's likely that this problem is similar to the problem described here https://buddypress.trac.wordpress.org/ticket/880 - the characters are being converted to unicode, and then being improperly truncated at some point.

However, I was unable to reproduce this in my own tests. When I used the group forum name suggested in your description, all links work as expected. Here are the places I'm testing:

  • example.com/members/boone/forums/
  • example.com/activity
  • example.com/groups/my-group/

The "2 minutes ago" links are pointing to the correct link in all cases.

A couple questions for you:

  1. You talk about a "freshness column" in "Sitewide Activity". Can you clarify? The format of the activity stream (example.com/activity) does not have a separate "freshness" column. Instead, "2 minutes ago" appears just after the text "boone gorges started the topic Αυτός είναι ένας εξαιρετικά μεγάλος τίτλος in the forum My Group". Does this match what you're seeing? if you're seeing a separate column here - do you have any plugins or other modifications that are changing the way that your activity page looks?
  1. Can you describe your server configuration a little more? What version of PHP? What encoding do you have on your wp_posts and wp_bp_activity tables (http://stackoverflow.com/questions/6103595/how-to-check-if-mysql-table-is-utf-8-and-has-storageengine-innodb)
  1. If possible, go into your database and find the following. (a) In the row in your wp_posts table corresponding to the forum post, get the 'post_name' value. (b) In the corresponding row in wp_bp_activity, get the whole row. Share those values here so that we can compare.

Thanks for your help debugging!

#2 @Cartographer
6 years ago

  • Keywords reporter-feedback removed

Hi boonebgorges,

Thank you very much for trying to find out what is going on.

I ll try to be more specific.

1) I create the Group Forum Topic: Αυτός είναι ένας εξαιρετικά μεγάλος τίτλος in the Group: Αυτό είναι ένα Group
So the final result is the following URL that is functional:
http://example.com/groups/αυτό-είναι-ένα-group/forum/topic/αυτός-είναι-ένας-εξαιρετικά-μεγάλος-τίτλος

P.S. As you noticed the Group name is in Greek as well.

In the Sitewide Activity every link is ok except the "2 minutes ago" part where the link is somehow truncated.

P.S. I was mistaken calling "Freshness column" this part. It is just the " x minutes ago" part that appears just after the text "user started the topic..."

Some more info:
PHP Version: 5.3
wp_posts and wp_activity_tables: utf8_general_ci (I didn't find something else)

I couldn't find what you are asking in the third point [(a) In the row in your wp_posts table corresponding to the forum post, get the 'post_name' value. (b) In the corresponding row in wp_bp_activity, get the whole row] but I will continue searching for these.

I hope I was much clearer now. Thank you again.

#3 @boonebgorges
6 years ago

  • Milestone changed from Awaiting Review to 2.2

Thanks very much for the clarification, Cartographer. Using your suggested group name, I was able to replicate (because a Greek group name plus a Greek topic name made the URL long enough to break things).

The issue is that the primary_link column in the activity table is VARCHAR(255). When the Greek characters are URL encoded to save to this field, it makes the URL longer than 255 characters.

Easiest (though perhaps the lamest) fix is to lift the 255 char restriction. There are a collection of issues on core Trac that are related; see eg https://core.trac.wordpress.org/ticket/29573. It would be nice to get someone with more multibyte and character-encoding-in-MySQL experience to chime in on our options aside from upping the 255 char limit. But let's do something for 2.2.

#4 @DJPaul
6 years ago

If we're failing to store a long string in a DB column because of a length limitation on that column, I don't see any alternatives to increasing the size of column (assuming we want the data to remain human-readable and easily query-able, and not be compressed somehow).

We've had this problem before, actually. In BP 1.6, we changed the length from 150 up to 255 to avoid truncation; r5966 #4118. :)

primary_link isn't indexed, so it shouldn't be a problem to change the column type. Suggest we change this to a text so we never run into the problem again.

#5 @boonebgorges
6 years ago

  • Keywords needs-patch added

We've had this problem before, actually. In BP 1.6, we changed the length from 150 up to 255 to avoid truncation; r5966 #4118. :)

Ha, good call. I'd forgotten.

primary_link isn't indexed, so it shouldn't be a problem to change the column type. Suggest we change this to a text so we never run into the problem again.

Agreed. Good call.

#6 @Cartographer
6 years ago

Hi there,

By changing the column type to text, do you mean that the "2 minutes ago" part won't be link any more? In my held opinion it has to be link.

I am not sure what do you mean so I would like to understand.

Thank you

#7 @boonebgorges
6 years ago

Cartographer - It will remain a link in the interface. We are discussing a possible change in the database schema, which will not affect the interface (aside from fixing this bug!)

#8 @DJPaul
6 years ago

  • Keywords good-first-bug added

#9 @tharsheblows
6 years ago

  • Keywords has-patch added; needs-patch good-first-bug removed

Changed primary_link in wp_bp_activity to text from varchar(255) and bumped version

@tharsheblows
6 years ago

change primary_link and bump version

#10 @djpaul
6 years ago

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

In 9168:

Core, Activity: change the Activity table's primary_link column from varchar(255) to text.

This change will avoid unintended truncation for long URLs. This can be a problem for links
built from slugs written in double-byte languages, or for letters that have been URL encoded.

We've tried previously to fix this, in r5966. Better luck this time. :)

Fixes #5890, props tharsheblows

#11 @johnjamesjacoby
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The speed difference for non-innodb setups between text and varchar may make this worth reverting. Can we get some benchmarks to ensure this is the best approach?

For installations where upper Unicode characters are common, switching to utf8mb4(_unicode_ci) and/or a larger varchar may be faster.

Also, it can be a slippery slope to cherry-pick what columns we change to support Unicode, and I suspect there are a few more places this will occur that should be checked into.

#12 @johnjamesjacoby
6 years ago

Maybe the more important decision is whether or not we prioritize (or actively maintain support for) non-innodb setups.

#13 @boonebgorges
6 years ago

Maybe the more important decision is whether or not we prioritize (or actively maintain support for) non-innodb setups.

Obviously we should continue to support it (in the sense that we should ensure that things continue to work), but I don't think we need to worry about *optimizing* for it. In particular, we definitely should not avoid fixing bugs (as in the current case) just out of concern about performance on MyISAM, as long as the performance degradation is not unreasonably large.

That being said, if there were significant speed improvements to be had by moving to a larger max varchar value as opposed to text, it'd be worth considering, as it would at least mitigate the bug currently being reported.

#14 @DJPaul
6 years ago

  • Keywords has-patch removed
  • Milestone changed from 2.2 to Future Release

I am going to move this into Future Release while interested people discuss the merits of changing the text type. As far as I'm concerned, this seems to me like premature optimisation; you could probably make a similar argument for a whole bunch of different columns across BuddyPRess and WordPress. KISS IMO :)

#15 @DJPaul
6 years ago

  • Milestone changed from Future Release to 2.2

Actually, I think it's best to keep in 2.2, so we can get accurate trac reports in the future.

#16 @Cartographer
6 years ago

Sorry for interrupting your technical conversation, but I would like to underline how important is this issue for us (the non-latin) speakers to be solved. Even the links in the bp email notifications are "broken/truncated" and the members who click on them are directed in error pages. I hope in 2.2 there will be a solution.

Thank you very much

#17 @DJPaul
6 years ago

Cartographer, don't worry, the fix for this is already in 2.2.

#18 @r-a-y
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

I'm going to close this since a fix for this issue is in.

We can talk about optimizing the DB column in another ticket.

Note: See TracTickets for help on using tickets.