WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 4 years ago

#22650 closed enhancement (fixed)

Filter the Gravatars on credits.php

Reported by: miqrogroove Owned by: wonderboymusic
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: Help/About Keywords: has-patch
Focuses: Cc:
PR Number:

Description

All Gravatars should be generated by get_avatar() or at least run through apply_filters('get_avatar', $avatar) for the sake of extensibility.

Related: #22329

Attachments (5)

credits.php.patch (1.1 KB) - added by miqrogroove 7 years ago.
22650.diff (1.7 KB) - added by GlennM 6 years ago.
22650-2.diff (1.2 KB) - added by GlennM 6 years ago.
22650.3.diff (3.4 KB) - added by SergeyBiryukov 5 years ago.
22650.4.diff (2.4 KB) - added by miqrogroove 4 years ago.

Download all attachments as: .zip

Change History (22)

#1 @miqrogroove
7 years ago

  • Keywords has-patch needs-testing added

#2 @GlennM
6 years ago

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

Tested on 4.0-alpha-src, added documentation for apply_filters and changed code according to WordPress coding standards.

@GlennM
6 years ago

#3 @GlennM
6 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I've attached a patch at WordCamp Netherlands Contributor Day.

#4 @ocean90
6 years ago

  • Component changed from Administration to Help/About
  • Milestone changed from Awaiting Review to 4.0

Hello GlennM, thanks for your patch. One thing I have noticed is the doc block for the filter, which is not needed. Take a look at http://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/#4-1-duplicate-hooks on how to handle this case.

@GlennM
6 years ago

#5 @GlennM
6 years ago

Thanks for your reply ocean90. I've updated my patch.

#6 follow-up: @SergeyBiryukov
6 years ago

The current patches here pass email hash as $id_or_email parameter, which should only be a user ID, email address, or comment object: tags/3.9.1/src/wp-includes/pluggable.php#L2113.

This can potentially break any custom code that only expects those three types of values.

I don't see an easy solution here, unless the API endpoint can be modified to return email addresses as well, which I guess is not feasible for privacy reasons.

#7 in reply to: ↑ 6 @miqrogroove
6 years ago

Replying to SergeyBiryukov:

The current patches here pass email hash as $id_or_email parameter, which should only be a user ID, email address, or comment object

That is actually a documentation error. $id_or_email is an optional parameter.

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.


5 years ago

#9 @SergeyBiryukov
5 years ago

  • Milestone changed from 4.0 to Future Release

All Gravatars should be generated by get_avatar() or at least run through apply_filters('get_avatar', $avatar) for the sake of extensibility.

What would be a use case for changing the Gravatars on Credits page?

#10 @SergeyBiryukov
5 years ago

Oh, I missed the link to #22329. Makes sense.

#11 @SergeyBiryukov
5 years ago

  • Keywords commit added; needs-testing removed
  • Milestone changed from Future Release to 4.0

I guess we should allow for get_avatar() to accept MD5 hash of an email address. See 22650.3.diff.

#12 @nacin
5 years ago

  • Milestone changed from 4.0 to Future Release

get_avatar() is frustratingly pluggable which means we might actually break this screen by doing this.

I'd like to un-plug this function, introduce get_avatar_url(), and such, but that's not going to happen overnight. Suggesting we wait a bit longer here to do this in conjunction with #21195.

#13 @SergeyBiryukov
5 years ago

Yeah, I've found quite a few plugins in the repository replacing get_avatar():

0gravatar/index.php
add-local-avatar/avatars.php
alkivia/components/gallery/gallery.php
author-or-user-image/Author___Image.php
baw-gravatar-google-image/bawggi.php
bbpress-social-network/ln_livenotifications.php
claimedavatar/claimedavatar.php
default-gravatar-sans/default-gravatar-sans.php
expire-password/pluggable.php
incarnate-for-wordpress/incarnate-for-wordpress.php
nnd-facebook-profile-for-gravatar/NNDFBProfGrav.php
post-author/post_author.php
presstest/mocked/core.php
sem-author-image/sem-author-image.php
simple-intranet-directory/avatars.php
simple-intranet-directory/profiles.php
simple-intranet-directory/profiles_lite.php
simple-local-avatars/simple-local-avatars.php
simple-schools-staff-directory/avatars.php
smartava/smartAva.php
wp-notifications/ln_livenotifications.php
wp-symposium/wp-symposium.php
xmpp-auth/plugged.php

#14 follow-up: @miqrogroove
4 years ago

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

Just need to implement nacin's ideas in a new patch.

#15 in reply to: ↑ 14 @obenland
4 years ago

Replying to miqrogroove:

Just need to implement nacin's ideas in a new patch.

That needs to happen very soon. Beta is two weeks away and there has been no traction on this ticket in almost a year.

@miqrogroove
4 years ago

#16 @miqrogroove
4 years ago

  • Keywords has-patch added

#17 @wonderboymusic
4 years ago

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

In 32845:

Make sure that all Gravatars are generated by get_avatar() or at least run through apply_filters( 'get_avatar', $avatar ) for the sake of extensibility.

Props miqrogroove, GlennM, SergeyBiryukov.
Fixes #22650.

Note: See TracTickets for help on using tickets.