get_avatar_url

[pathawks]

There should be a separate function to just get an avatar URL, versus getting the image wrapped in an <img> tag. This function should pass the avatar URL through a filter before returning it.
get_avatar would then call this function, rather than generating a Gravatar URL on its own.

This way, a plugin can grab a user's avatar image without relying on regex to extract the image from the <img> tag, and in a way that is compatible with plugins that replace Gravatar.

[c3mdigital]

Wouldn't the get_avatar filter work for this?

[pathawks]

Get get_avatar filter works great when you want the full <img> tag.
I'm after a function & filter that applies only to the image URL.

I realize that I could regex the image URL from the <img> tag, but what if a plug-in has replaced get_avatar() in such a way that it no longer returns images? What if I want to include avatars as background with CSS rather than in an <img> tag?

[Ipstenu]

This stems from ​http://wordpress.org/extend/ideas/topic/why-is-avatar-stuff-in-pluggablephp

Honestly I don't know why you'd want to get the avatar image and not put it in an image tag. What are you doing with it if not displaying it? The whole point of the avatar is to show it, and for that you'll always need the img tags.

Which I asked in the other thread too :)

[toscho]

@Ipstenu You could use the avatar as a background image for example.

[readysetawesome]

+1

This is an issue any time you want to use avatar images outside the context of displaying avatars in an HTML page, which is IMO a reasonable thing to want to do. The use case necessitates really ugly parsing code, and seems like it could be much more cleanly handled with some core code changes.

[batmoo]

Patch breaks URL-building piece of get_avatar into a new function called get_avatar_url.

[pathawks]

Beautiful!

[mdgl]

This would be useful also for people who just want to add standard HTML attributes to the <img> tag for a gravatar, for example "title" (for tooltip), "id" or further "class" names.

You could argue, however that these simple cases should be catered for directly by get_avatar() with additional optional parameters in the same way as "alt".

To do this kind of thing at the moment you need to either replace the function entirely or parse the returned <img> tag in a filter. Both methods are messy and error prone.

[SergeyBiryukov]

Related: #21930

[mdawaffe]

attachment:21195.3.diff​ refreshes the patch and adds some functionality we've found useful at WordPress.com.

attachment:21195-ut.3.diff​ provides new test coverage for get_avatar() and get_avatar_url(). All test pass for the attached patch in PHP5.3 and PHP5.4. I don't have a handy way of testing under PHP5.2 right now.

• Get avatar by user_id, user (new), email address, MD5 hash (new), comment, or post (new).
  • Getting by MD5 is handy when you have a hash but no the email address.
  • Getting by user or post is not strictly necessary, but provides a little extra context to plugins hooking in to various hooks.
• Support for all Gravatar parameters:
  • size
  • default (404, retro, monsterid, wavatar, identicon, mysteryman, blank, gravatar logo, and URL)
  • force_default
  • rating
• pre_get_avatar_url, get_avatar_url, pre_get_avatar, get_avatar filters (see #21930)
  • The first three of which can modify the $args parameter on the fly (it's passed by reference to the filter) allowing plugins to modify default behavior without needing to duplicate code.
• Uses set_url_scheme()
• Updates to latest Gravatar supported subdomains.
• Argument to add classes to the returned img element.
• Argument to force the return of the img element, ignoring the show_avatars option.
• Fixes some edge case bugs
  • Non integer and non-positive sizes
  • Negative user IDs
  • Comments by users who's accounts have since been deleted

Implementation Notes

With the patched filters, I don't think get_avatar_url() needs to be pluggable.

Both functions now accept a single $args array parameter instead of many parameters. This makes it easier for plugins to extend the functionality or replace it with a service that doesn't have the same set of parameters.

In the attached patch, get_avatar() respects the show_avatars option (as it does currently) unless the new force_display argument is true. In contrast, get_avatar_url() ignores the show_avatars option and always returns a URL if it can.

If the default avatar is a URL, get_avatar() currently appends the requested size to the default URL as an s={$size} query string parameter. In the patch, this behavior is removed since such a parameter is not needed (and is ignored) by Gravatar when serving default images. In theory, that removal could break some avatar-substituting plugins. I'm ambivalent: removing it cleans up the code a bit, but we can add it back.

If get_avatar() can't figure out an email address to hash, it currently uses unknown@gravatar.com, yielding URLs like: http://www.gravatar.com/avatar/ad516503a11cd5ca435acc9bb6523536?s=96. The attached patch instead does not include any hash in the URL for these cases: http://www.gravatar.com/avatar/?s=96.

If get_avatar() can't figure out an email address to hash but has a default image URL, it currently uses that URL as the src of the img: src="http://example.com/default.jpg". The attached patch instead sends that URL through Gravatar so that it can be sized correctly: src="http://www.gravatar.com/avatar/?s=96&d=http://example.com/default.jpg", which could break some URLs (since the provided default URL must be accessible to Gravatar for the image to be served). I chose this behavior since it matches what happens when we can find an email address to hash, but when that hash has no associated Gravatar.

Getting a URL by MD5 as implemented requires you to pass "{$md5}@md5.gravatar.com" in the first parameter. I did this because some hashes will pass the is_numeric() test. Who can say? Maybe someday a WordPress site will have more than 10 nonillion user accounts :)

In addition to the URL returned by get_avatar_url(), get_avatar() needs some other information calculated by get_avatar_url(): the sanitized avatar size, whether a potential avatar match was found, and potentially more by/for the various filters. Rather than calculating things twice or adding an intermediary function, get_avatar_url()'s args array accepts a 'processed_args' item, which should be passed by reference. get_avatar_url() fills that reference with the processed args/flags, and get_avatar() reads it.

I chose to implement this "out variable" as $args['processed_args'] rather than get_avatar_url( $id_or_email, $args, &$processed_args ) for selfish reasons: versions of Jetpack have shipped with code like:

get_avatar_url( 1, 96, '' );

which will generate a Fatal Error if get_avatar_url()'s signature has a reference as the third parameter.

[cliffseal]

I was working with this today, but went a little different route by adding an extra parameter to the get_avatar function allowing you to just return the URL. If this has been explored already, forgive me, but I couldn't find any discussion on it. Thoughts?

[cliffseal]

Add parameter to get_avatar

[pathawks]

Replying to cliffseal:

I was working with this today, but went a little different route by adding an extra parameter to the get_avatar function allowing you to just return the URL.

The biggest problem I see with this approach is that, because get_avatar is pluggable, some plugins replace the entire function.

If get_avatar has been plugged with a version that does not support returning just the URL when given an extra paramater, it will break anything that relies on that behavior.

Additionally, this seems inconsistent with how functions like wp_get_attachment_image and wp_get_attachment_image_src work, with one function returning the whole <img> tag and another returning just the URL.

[cliffseal]

Replying to pathawks:

If get_avatar has been plugged with a version that does not support returning just the URL when given an extra paramater, it will break anything that relies on that behavior.

I wasn't aware this would happen. It doesn't occur in any of my tests and seems unintuitive, but it's quite possible you know something I don't.

Additionally, this seems inconsistent with how functions like wp_get_attachment_image and wp_get_attachment_image_src work, with one function returning the whole <img> tag and another returning just the URL.

True, but neither of those functions are pluggable, so I'm not sure if the correlation should determine the method.

Regardless, sharing what worked for me here. I'll be pleased with whatever the best solution is.

[pathawks]

Replying to cliffseal:

Replying to pathawks:

If get_avatar has been plugged with a version that does not support returning just the URL when given an extra paramater, it will break anything that relies on that behavior.

It doesn't occur in any of my tests

Of course it doesn't; you're working with a modified version of get_avatar :)

The problem comes when a plugin has replaced get_avatar completely. In that case, no updates that have been made to core's get_avatar will be reflected.
In this case, get_avatar would never return just a URL, because it was merely designed to replace the legacy get_avatar.

Now, the problem comes when a plugin expects to be able to retrieve a URL from get_avatar. As you can see, it wouldn't be possible in this senario. If the new plugin is expecting just a URL to be returned and instead is given an <img> tag, Bad Things will happen. If the plugged get_avatar is passed more parameters than it knows what to do with, Bad Things might happen.

[cliffseal]

Replying to pathawks:

Of course it doesn't; you're working with a modified version of get_avatar :)

Just to be super clear here: no, I'm not. I kept the current version of get_avatar in pluggable.php and overrode it in a plugin for testing.

I think I see what you're saying, though. Plugins conflicts could occur if one plugin is overriding the function and another plugin is trying to call the updated version of the function—but, it would seem any decent plugin author would check for this error before proceeding.

[pathawks]

Replying to cliffseal:

Replying to pathawks:

Of course it doesn't; you're working with a modified version of get_avatar :)

Just to be super clear here: no, I'm not. I kept the current version of get_avatar in pluggable.php and overrode it in a plugin for testing.

Sorry, should have been more clear. WordPress is using a modified version of get_avatar, at any rate.

It would seem any decent plugin author would check for this error before proceeding.

If that's the case, then what would be the point in changing things?
If I will have to, not only provide a way to extract the URL from the returned <img> tag in these circumstances, but also check to see if the output from get_avatar is a URL or a tag, we actually end up with far more code than what we would currently need.

No, it seems that, if anything changes, a new function should be added called get_avatar_url or whatever, and get_avatar should be left alone. I see absolutely no advantage from trying to shim totally different functionality into an existing function, which currently does it's job well enough.

[pathawks]

Replying to mdawaffe:

I have ​packaged mdawaffe's patches into a plugin, if that makes it easier for others to test.

[rmccue]

I've ​included this as a separate function in my JSON API plugin; this is ripe for core inclusion in some form. I think a separate function would be a better choice here.

[pento]

attachment:21195.diff​ updates mdawaffe's patch to apply cleanly against trunk.

[pento]

attachment:21195.2.diff​​ updates the inline docs to more closely resemble the new standard.

[rmccue]

Replying to pento:

attachment:21195.2.diff​​ updates the inline docs to more closely resemble the new standard.

I want me some of this.

I think your case statements should be indented a block, as with the } in the default PHPDoc.

[wonderboymusic]

I use the same style for cases, which is the PEAR standard and the style that validates in jslint

[pento]

attachment:21195.4.diff​​ fixes the case and PHPDoc indentation.

Looking through a handful of core files, this seems to be the correct indentation for case statements (thought there's variation over whether there should be a space before the : or not).

[rmccue]

Replying to wonderboymusic:

I use the same style for cases, which is the PEAR standard and the style that validates in jslint

We should codify this in the standard. Here's the stats for *.php:

• ^(\t+)switch.*\n\1case => 17 matches
• ^(\t+)switch.*\n\1\tcase => 278 matches

And for `*.js*:

• ^(\t+)switch.*\n\1case => 2 matches (both in nav-menu.js)
• ^(\t+)switch.*\n\1\tcase => 18 matches

[rmccue]

Replying to pento:

attachment:21195.4.diff​​ fixes the case and PHPDoc indentation.

Looking through a handful of core files, this seems to be the correct indentation for case statements (thought there's variation over whether there should be a space before the : or not).

For *.php:

• case .*[^ ]: (no whitespace) => 2339 matches
• case .* : (whitespace) => 286 matches

(This is less of a standard though; it's more about the context. We might want to standardise this too though.)

[dd32]

function get_avatar( $id_or_email, $args = null )

I don't think we can change the signature like that since it's a pluggable function, adding optional parameters is OK if the calling code can deal with it not being respected, but we should attempt to limit any in/out behaviour of the existing function.

Moving the entire contents of the function to a get_avatar_url() function is perfectly fine though, just have to remember that anyone who has a pluggable get_avatar() at present will suddenly have their functions alterations not respected.

[mdawaffe]

Replying to dd32:

I don't think we can change the signature like that since it's a pluggable function

The patched function can accept the old signature, but I see what you're saying. If we change the signature, new calls in core and plugins will use the new signature, which will break for sites which have a plugged version with the old signature.

[pento]

attachment:21195.5.diff​

• Changes get_avatar() back to the old signature, with a new optional fifth $args argument, and changes the PHPDoc to match
• Re-adds some arguments that were accidentally removed from the PHPDoc for get_avatar()
• Fixes a PHP warning when there's no email hash available

[SergeyBiryukov]

Related: #27336

[wonderboymusic]

If someone refreshes this, we can look and possibly move to 4.0

[rmccue]

For reference, ​here's how we do it in WP-API currently. (Basically, regex on HTML, bleh.)

[pathawks]

Refresh patch

[pathawks]

Refresh

[pathawks]

21195.7.diff Refreshes the patch.

What is next?

[rachelbaker]

Current approach used in the JSON REST API

[SergeyBiryukov]

The approach in 21195.patch​ looks more like a workaround. We don't want to add new pluggable functions, so 21195.7.diff​ seems like the way to go.

#22650 would benefit from this.

[pathawks]

Because get_avatar is pluggable, we cannot safely assume that it will always even return an <img> tag. It could, for instance, return a <div> with a backgroud set to an avatar image, or something silly.

Because of the (albeit remote) possibility of this, running a regex on the return from get_avatar only works well when you are certain that nothing odd has plugged get_avatar. A safe assumption for WordPress.com, perhaps not a safe assumption for the rest of the world.

What would be the possible advantage of adding a new function to pluggable.php? Why not just send output (Gravatar URL) through a filter?

[wonderboymusic]

@pento, @mdawaffe, @SergeyBiryukov - one of you please remind yourself why you fell in love with get_avatar_url() in the first place

[wonderboymusic]

#27336 was marked as a duplicate.

[pento]

That was a fun walk down memory lane. 21195.8.diff​:

• Updates the patch to apply cleanly against trunk
• Re-adds some filter PHPDocs that had been removed
• Adds new PHPDocs for new filters
• Adds PHPDocs for new parameters to old filters
• Fixes a bunch of space/tab indent issues
• Fixes some minor coding style issues

It wouldn't kill us to put some unit tests on this, too. I'll do that sometime later today, if someone doesn't beat me to it.

@DrewAPicture - I couldn't find anything in the doc standards about marking when a new parameter was added, just a couple of examples that I managed to grep for the source. I went with the (since 4.1.0) style, please correct me if there's a better way. :-)

[DrewAPicture]

docs pass 2

[DrewAPicture]

21195.9.diff​ brings the docs in-line with the standards, retaining most of the awesome background information supplied by @pento in .8.

[pento]

21195.10.diff​:

• Adds unit tests for get_avatar() and get_avatar_url()
• Fixes get_avatar_url() failing if it's passed the array version of a user/post/comment object
• Fixes get_avatar_url()'s $args parameter not being passed as a reference
• Fixes get_avatar_url() returning a broken URL if it couldn't figure out the email hash
• Tweaks the docs just a little more

[DrewAPicture]

#21930 was marked as a duplicate.

[bgerhardt1488]

Replying to pathawks:

There should be a separate function to just get an avatar URL, versus getting the image wrapped in an <img> tag. This function should pass the avatar URL through a filter before returning it.
get_avatar would then call this function, rather than generating a Gravatar URL on its own.

This way, a plugin can grab a user's avatar image without relying on regex to extract the image from the <img> tag, and in a way that is compatible with plugins that replace Gravatar.

[pento]

21195.11.diff​ updates the patch to apply cleanly against trunk, and updates the @since version numbers.

[pento]

21195.12.diff​ replaces all of the apply_filters_ref_array() calls with apply_filter() - I was unable to find a difference between the behaviour of them.

[pento]

So, I realised the purpose of the apply_filters_ref_array() - to allow filters to change the $args.

Because references make everyone sad, 21195.13.diff​ shuffles all of the $args processing into a new get_avatar_data() function, with filters specifically for filtering the data. get_avatar_url() is now just a little wrapper around this function.

This has the added bonus of allowing us to remove the $processed_args references, as get_avatar() is able to call get_avatar_data(), and get all of the processed args directly.

[pento]

In 21195.14.diff​:

• Drop support for the array versions of WP_User / WP_Post objects - it's a new feature so we don't need to support the old way of doing things.
• Moved the get_avatar_url filter to get_avatar_data().
• Dropped $original_args from all of the filters, it didn't really have a practical purpose.

[pento]

Does anyone have opinions on 21195.15.diff​, which deprecates the get_avatar() pluggable function, in favour of wp_get_avatar()? (Currently just a copy/pasta, but we could make this code a lot neater if we didn't need to care about function signatures, for starters.)

[pento]

In 31107:

Add get_avatar_url(), for retrieving just the URL of an avatar, rather than the entire <img> tag that get_avatar() produces.

Unlike get_avatar(), get_avatar_url() is not pluggable. It can be extended/or modified through the new filters included.

Fixes #21195.

Props mdawaffe, pento, pathawks, DrewAPicture

[valendesigns]

When viewing options-discussion.php all generated avatars are replaced with your personal avatar. Patch 21195.16.diff​ fixes this by using the new $args array and setting force_default to true. I also removed some filters that were added to force avatars to display on options-discussion.php and replaced them by setting the force_display option to true.

Unfortunately, another side effect of the newly implemented get_avatar means that the Mr. WordPress avatar is no longer showing in comments on the front end because it does not have an email address attached to the comment. That was not the behavior previously and we should fix this regression.

[obenland]

valendesigns, the next line after the avatar is retrieved, where the img src is being replaced, can also be removed.

[nacin]

Replying to valendesigns:

When viewing options-discussion.php all generated avatars are replaced with your personal avatar. Patch 21195.16.diff​ fixes this by using the new $args array and setting force_default to true.

Why did the existing code break? preg_replace("/src='(.+?)'/", "src='\$1&forcedefault=1'", $avatar);

I also removed some filters that were added to force avatars to display on options-discussion.php and replaced them by setting the force_display option to true.

One problem with the approach we have here is there's no real way of knowing if these arguments work for get_avatar().

I'm thinking about an alternate approach that looks something like this:

<?php
function wp_get_avatar( ... ) {
    if ( defined( 'PLUGGED_GET_AVATAR' ) && empty( $args['force_core_function'] ) ) {
        return get_avatar( ... );
    }
    // else, the rest of the function
}

# pluggable-deprecated.php
if ( function_exists( 'get_avatar' ) ) {
    define( 'PLUGGED_GET_AVATAR', true );
    _deprecated_function( 'get_avatar', ... ); // optional. need to pass a message of some sort.
} else {
    function get_avatar( ... ) {
        return wp_get_avatar();
    }
}

That way we can finally banish get_avatar(), use wp_get_avatar() everywhere in core, force it to use core functionality when we need it, but not break anything overriding get_avatar() deliberately. We could possibly use this as a template for deprecating, eventually, all pluggable functions. Any thoughts here?

Unfortunately, another side effect of the newly implemented get_avatar means that the Mr. WordPress avatar is no longer showing in comments on the front end because it does not have an email address attached to the comment. That was not the behavior previously and we should fix this regression.

Indeed, this is broken.

[obenland]

Why did the existing code break?

It went from src='' to src="".

[jeremyfelt]

Replying to nacin:

We could possibly use this as a template for deprecating, eventually, all pluggable functions. Any thoughts here?

In the past, we've deprecated without making the promise that core will continue to use the plugged version [18513]. If all instances in core would use the force_core_function argument, it seems unnecessary to continue looking for a plugged get_avatar().

That said, I'm also not familiar with issues that may have arisen from previous deprecated pluggables. If there are reasons for core to use wp_get_avatar() without forcing, then the above code looks sane for maintaining back-compat.

Looking around, #19615 could also be a good test (mostly because it's open) for deprecating pluggable get_currentuserinfo().

[nacin]

In [18513]'s case, get_user_by() is also pluggable. Presumably, if one is plugging the user_by functions, they're plugging all of them.

Frankly, I care very little about the promise of most pluggable functions. The auth ones, sure. But then there's get_avatar(), a fairly high-level function that, if I had to take a guess, is commonly overridden to use something other than Gravatar. I wish it were never pluggable and that it simply had just the filter (which was introduced at the same time).

I don't know — maybe we can get away with ditching it, deprecating it, and just using a new wp_get_avatar(), and anyone who is currently overriding get_avatar() just gets screwed and told they should have been using the filter.

[jeremyfelt]

Replying to nacin:

I don't know — maybe we can get away with ditching it, deprecating it, and just using a new wp_get_avatar(), and anyone who is currently overriding get_avatar() just gets screwed and told they should have been using the filter.

Would there be a wp_get_avatar() use in core where we wouldn't use force_core_function to override?

Though... providing the check for the pluggable at least announces the deprecation, even if core itself skips it. This could help push developers to transition more than just moving to wp_get_avatar() and leaving behind a deprecation notice that may never fire.

[pento]

In 31152:

In get_avatar(), revert the <img> tag attributes to using single quotes, instead of double quotes. This behaviour was changed in [31107], but caused problems for code that attempted to parse the <img> tag.

See #21195

[pento]

In 31153:

In get_avatar_data(), there's no need to return false if we couldn't find an avatar, as Gravatar can handle being given an empty email hash. This allows the default avatar to show when no email address is given.

See #21195

[pento]

In 31154:

If get_avatar_data() is passed an empty value for the default avatar, we should be using the site's avatar_default option instead.

See #21195

[dshanske]

Isn't this a chance, while reviewing get_avatar and get_avatar_url, to add built-in support for local avatars which was proposed as a feature plugin? I've always felt that polling an external service for local accounts seemed wrong.

[pento]

Adding local avatar support is waaaaay outside of the scope of this ticket. :-)

The best place for that would be to get the proposed feature plugin up and running, then we can look at merging it later.

[SergeyBiryukov]

In 31480:

Remove src from duplicate hook comments for get_avatar and get_avatar_data.

see #21195.

[pathawks]

This ticket was mentioned in Slack in #core by jip. View the logs.

Apparently it is impossible for a non-member to view the logs? Lovely.

[ocean90]

Replying to pathawks:

Apparently it is impossible for a non-member to view the logs? Lovely.

You can be a member too, see ​https://make.wordpress.org/chat/.

[pathawks]

Replying to ocean90:

You can be a member too, see ​https://make.wordpress.org/chat/.

Thanks missed that somehow.