Skip to:
Content

Opened 20 months ago

Last modified 5 months ago

#4458 reopened defect (bug)

Activity update deletion and the "Are you sure" dialog box

Reported by: tifire Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Activity Keywords: dev-feedback needs-patch
Cc:

Description

I assume the "Are you sure" dialog box is required for every activity update deletion, so I think this is a buddypress bug.

I have tested this on testbp.org. Here is what I found:
1) Right after posting an activity update, no "Are you sure" dialog box shows up if I click the "delete" button. The activity update is just deleted.
2) After posting an activity update, if I refresh the page then click the "delete" button, the "Are you sure" dialog box shows up for me to confirm the deletion.
3) If I refresh the page, select any of the filter ( everything, updates, new groups, group memberships, etc), all my activity updates can be just deleted without seeing the "Are you sure" dialog box.

Attachments (1)

4458.diff (1.2 KB) - added by boonebgorges 18 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 DJPaul20 months ago

  • Milestone changed from Awaiting Review to 1.7
  • Severity changed from critical to normal

Confirmed on testbp which is currently running 1.6.

comment:2 boonebgorges20 months ago

  • Keywords dev-feedback added

What's our policy on fixing this kind of issue for bp-default/bp-legacy?

comment:3 DJPaul18 months ago

The issue is the jQuery is bound to DOM elements that exist at page load time. Elements added in later aren't bound to the click detection, so our message box doesn't appear. If this can be fixed without a huge rewrite of BP-Default's JS, I say we should do it -- ideally we not to ought have differing behaviour for the "same" button.

comment:4 boonebgorges18 months ago

Right. In the rest of the activity stream, we use proper delegation. But in the case of our .confirm js, it's printed in the document head, with bp_core_confirmation_js(). Something like 4458.diff should work, in theory. I had to change the priority to 1 so that it was printed before the main site scripts, otherwise it wouldn't fire in time. Still, there's a race condition somewhere that's causing the confirm script not to fire in all cases. If anyone has any bright ideas....

FWIW this is not a bp-default issue, as we print this bit of script in core.

boonebgorges18 months ago

comment:5 johnjamesjacoby17 months ago

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

(In [6597]) Fix bug with confirm JS not working on AJAX refreshed activity streams. Props boonegorges. Fixes #4458.

comment:6 boonebgorges14 months ago

(In [6809]) Don't use event delegation in bp_core_confirmation_js()

Event bubbling prevents the JS confirm dialog from working properly.

This commit reverts changes introduced in #6597. See #4458

Fixes #4843. Props sbrajesh

comment:7 boonebgorges14 months ago

  • Milestone changed from 1.7 to Future Release
  • Resolution fixed deleted
  • Status changed from closed to reopened

The delegation solution won't work globally. See #4843. We're going to have to live with this bug for now.

comment:8 vhauri5 months ago

This could probably be fixed by changing some return false; statements to e.preventDefault();

For example, line 169 of bp-themes/bp-default/_inc/global.js uses a return false; which will definitely break bubbling, but that's exactly what the jQuery method's for. If this is a lost cause, please let me know, otherwise I'll happily whip up a quick patch.

comment:9 boonebgorges5 months ago

  • Keywords needs-patch added

vhauri - Absolutely, please do write a patch. Could you please patch against bp-templates/bp-legacy/js/buddypress.js? bp-default is being EOLed.

Note: See TracTickets for help on using tickets.