Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

#6608 closed defect (bug) (fixed)

Groups invite friends input control echoing checked attr

Reported by: hnla's profile hnla Owned by: boonebgorges's profile boonebgorges
Milestone: 2.4 Priority: high
Severity: major Version:
Component: Groups Keywords: has-patch
Cc:

Description (last modified by DJPaul)

In trunk bp-groups-templates.php L: 4957 we are currently echoing the checked() function when selecting a name to invite checked="checked" checked() being an echo function!

This is introduced by way of changes to using the checked() function rather than passing in a variable as before; in changeset r9939

checked() takes two mandatory? args and a third optional $echo true/false we need to pass that third arg if using this function so patch re-writes the $checked variable to hold the name ID if in array or else empty it and updates checked to carry $checked against the inputs value and ads 'false' to prevent echoing.

Attachments (3)

6608-01.patch (1.2 KB) - added by hnla 8 years ago.
Update use of checked() function for invite name input control
6608-02.patch (1.2 KB) - added by hnla 8 years ago.
Updated patch removes the label enhancements from 6605 ticket - just includesthe checked fix
6608-checked.jpg (60.6 KB) - added by mercime 8 years ago.

Download all attachments as: .zip

Change History (9)

#1 @DJPaul
8 years ago

  • Description modified (diff)

@hnla
8 years ago

Update use of checked() function for invite name input control

#2 @DJPaul
8 years ago

The second parameter of checked() defaults to true. We don't gain anything by passing a string instead.
It looks like checked( $checked, true, false ) is the best smallest change to fix the issue this ticket covers.

RE: the label element you added in the patch and didn't mention: while this part of code has a number of stylistic and standards issues that I would probably change if I were writing this today, let's keep fixes and improvements separate. :)

#3 @hnla
8 years ago

hmm the intention was to separate the notion of the label element fix from the other ticket hence creating this ticket and removing the checked fix from the other patch, shouldn't have then updated this patch from the fixed label one I guess :(

second parameter of checked() defaults to true

Ah ok so it just needed to be stated even though a default to allow the third param to be set, cool!

Well this patch can be cleaned back to pre label update to keep things clearer.

@hnla
8 years ago

Updated patch removes the label enhancements from 6605 ticket - just includesthe checked fix

#4 @hnla
8 years ago

I can't get valid results from changing the second param on checked() so have left it at my implementation unless someone else wants to do something better.

#5 @mercime
8 years ago

@hnla nice catch on the 'checked' issue. I've attached a screenshot of the issue as seen on Twenty Fifteen.

@mercime
8 years ago

#6 @boonebgorges
8 years ago

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

In 10240:

Don't echo results of checked() when building group invite list.

Props hnla, DJPaul.
Fixes #6608.

Note: See TracTickets for help on using tickets.