Skip to:

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3429 closed defect (bug) (fixed)

BP_Embed tweaks

Reported by: r-a-y's profile r-a-y Owned by:
Milestone: 1.5 Priority: normal
Severity: minor Version: 1.5
Component: Core Keywords: has-patch


While working on my updated oEmbed plugin, I found a few more things that I missed.

Attached patch:

  • Properly caches activity comment embeds (due to the new activity comment template code that I forgot to test for)
  • Fixes notice in BP_Embed::shortcode() and improperly named filter in BP_Embed::parse_oembed()
  • Adds an action to pass the activity ID when expanding the truncated activity entry via AJAX. In this patch, I did not add the code to show the embed after expanding. Let me know if you'd like to see another patch with this included.

Attachments (3)

3429.01.patch (4.0 KB) - added by r-a-y 13 years ago.
3429.02.patch (4.6 KB) - added by r-a-y 13 years ago.
3429.03.patch (6.6 KB) - added by r-a-y 13 years ago.

Download all attachments as: .zip

Change History (12)

13 years ago

#1 @DJPaul
13 years ago

Regarding bullet point 3; I might be misunderstanding, but doesn't it do it already? See find the discussion with "This thread, r-a-y!" :)

Version 0, edited 13 years ago by DJPaul (next)

#2 @r-a-y
13 years ago

Hmm, you're right... let me try duplicating on a fresh install of BP 1.5-beta-1.

Can you check to see if that embed item is saved in the activity meta table?

13 years ago

#3 @r-a-y
13 years ago

Just tested.

What you're seeing on is the bug in point 2. The $id should be empty for an activity comment or an expanding activity item, but because $id is also defined above the code, BP_Embed thinks that the $id exists, but in actual fact it's a string of the embed handler, which isn't numeric.

The second patch makes this a little clearer.

When you patch just point 2, you'll see that embeds for recursive activity comments and expanded activity items will not show up.

The reason why is recursive activity comments and the expanding activity item do not use a real activity loop ( bp_has_activities() ). The current code in 1.5-beta-1 ( bp_activity_embed() ) only checks if an activity ID exists during the activity loop.

#4 @DJPaul
13 years ago

I'm pretty sure we don't want to put something into trunk that causes a regression on existing functionality, whether that was caused by a bug or not :)

13 years ago

#5 @r-a-y
13 years ago

Gotcha! New patch adds embed functionality when you expand long activity items via the "Read More" link.

Since this is theme-specific, I put that code in bp-default's functions.php.

#6 @DJPaul
13 years ago

  • Milestone changed from Awaiting Review to 1.5
  • Severity changed from normal to minor

#7 @djpaul
13 years ago

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

(In [4937]) Tweak BP_Embed implementation; fix caching and PHP warning. Fixes #3429, props r-a-y

#8 @DJPaul
13 years ago

Thanks for the patch, r-a-y. I made a couple of minor tweaks: the create_function() didn't work, and I decided to put the stuff you had added to functions.php into Activity core.

#9 @r-a-y
13 years ago

Thanks for adding the $activity object in "bp_dtheme_get_single_activity_content" action. I was about to suggest that!

One tiny thing, instead of using intval() maybe cast it instead?

Note: See TracTickets for help on using tickets.