Skip to:

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#4510 closed defect (bug) (fixed)

Activity stream posting and commenting broken

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 1.7 Priority: normal
Severity: normal Version:
Component: Activity Keywords: has-patch
Cc: boonebgorges


In trunk, activity stream commenting and posting is broken.

  • Hierarchical comments don't attach to the correct parent.
  • New activity updates and completed comments don't display the newly posted comment.
  • Triggering a few notices. Attached patch addresses them, and also repairs some funky JS selectors that I'm not sure ever worked.

It's possible something in theme compat is breaking this, though I'm not sure how.

Attachments (1)

4510.patch (1.7 KB) - added by johnjamesjacoby 12 years ago.

Download all attachments as: .zip

Change History (8)

#1 @boonebgorges
12 years ago

  • Keywords commit added

Theme compat itself is not breaking this, but something that got thrown in with theme compat is.

In r6285, the 'bp_after_theme_setup' hook was introduced, hooked to 'after_setup_theme' at priority 10. At that time, you changed bp_dtheme_register_actions() to fire on this new hook. However, since bp_dtheme_setup() continued to be hooked to after_theme_setup at priority 10, and bp_dtheme_setup() is the file where ajax.php (and bp_dtheme_register_actions()) is included, a race condition was created on after_setup_theme:10. As a result, the AJAX handlers in bp_dtheme_register_actions() are not being hooked to their wp_ajax_ actions.

The solution is to do one of the following:
1) Move bp_dtheme_setup() to bp_after_theme_setup
2) Move bp_dtheme_register_actions() back to after_theme_setup

I suggest (2). bp-default is a legacy theme now, and should not be changed. Moving to a new hook could cause backward compatibility issues for themes that load our ajax.php but not our functions.php. I'll commit this change if there are no objections.

The JS errors being noticed before need some more investigation. They don't actually seem to be part of the root problem of this ticket.

#2 @johnjamesjacoby
12 years ago

(In [6323]) Theme Compat:

  • Hardcode a bp-default check into bp-legacy functions to prevent duplicate loading of global.js.
  • See #4510.

#3 @johnjamesjacoby
12 years ago

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

(In [6326]) Theme Compat:

  • Prevent firing ajax actions inside buddypress-functions.php when bp-default is active in theme stack.
  • Revert part of r6285 and put bp-default actions back on after_setup_theme for backpat.
  • Repair some bp-default JS that never worked correctly.
  • Prevent debug notices in activity stream comment posting and notification sending if not AJAX.
  • Also fixes bp-legacy ajax activity comment posting.
  • Fixes #4510

#4 @DJPaul
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

About r6323. Doesn't handle for BP-Default being duplicated and renamed. It might be impossible to catch all cases but we could additionally check if the active theme has a "BuddyPress" tag in style.css or if the default javascript has already been enqueued.

#5 @johnjamesjacoby
11 years ago

  • Keywords 2nd-opinion commit removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Seems fixed with bp-default and bp-legacy. Closing as fixed.

#6 @mattryken
11 years ago

This seems to be an issue in the 1.6.x milestones as well. I've verified it in both 1.6.1 and 1.6.2. Is it possible to get these changes added to a 1.6.3 update?

#7 @r-a-y
11 years ago

(In [6704]) Backport bp-default JS fixes from r6326 to 1.6 branch.

See #4510.

Note: See TracTickets for help on using tickets.