Skip to content

Commit

Permalink
Allow custom default avatar icons
Browse files Browse the repository at this point in the history 
Following implementation of #13992, it was no longer possible for
administrators to define a custom default avatar, as Mantis forced use
of Gravatar's 'identicon'.

This commit reintroduces that functionality, and allows use of either
one of Gravatar's default images or a custom one (identified by an URL),
through appropriate configuration of $g_show_avatar.

Fixes #14032
  • Loading branch information
@dregad
dregad committed Aug 15, 2012
1 parent 6890b5a commit f710546
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 26 deletions.
20 changes: 16 additions & 4 deletions config_defaults_inc.php
View file
Expand Up @@ -936,12 +936,24 @@

/**
* Show user avatar
* the current implementation is based on http://www.gravatar.com
* users will need to register there the same address used in
* this MantisBT installation to have their avatar shown
*
* The current implementation is based on http://www.gravatar.com
* Users will need to register there the same email address used in this
* MantisBT installation to have their avatar shown.
* Please note: upon registration or avatar change, it takes some time for
* the updated gravatar images to show on sites
* @global int $g_show_avatar
*
* The config can be either set to OFF (avatars disabled) or set to a string
* defining the default avatar to be used when none is associated with the
* user's email. Valid values:
* - OFF (default)
* - One of Gravatar's defaults (mm, identicon, monsterid, wavatar, retro)
* @link http://en.gravatar.com/site/implement/images/
* - An URL to the default image to be used (for example,
* "http:/path/to/unknown.jpg" or "%path%images/no_avatar.png")
*
* @global int|string $g_show_avatar
* @see $g_show_avatar_threshold
*/
$g_show_avatar = OFF;

Expand Down
2 changes: 1 addition & 1 deletion core/print_api.php
View file
Expand Up @@ -144,7 +144,7 @@ function print_successful_redirect( $p_redirect_to ) {

# Print avatar image for the given user ID
function print_avatar( $p_user_id, $p_size = 80 ) {
if ( OFF == config_get( 'show_avatar' ) ) {
if ( OFF === config_get( 'show_avatar' ) ) {
return;
}

Expand Down
35 changes: 18 additions & 17 deletions core/user_api.php
View file
Expand Up @@ -794,28 +794,29 @@ function user_get_name( $p_user_id ) {
* @return array|bool an array( URL, width, height ) or false when the given user has no avatar
*/
function user_get_avatar( $p_user_id, $p_size = 80 ) {
$t_email = utf8_strtolower( trim( user_get_email( $p_user_id ) ) );
if( is_blank( $t_email ) ) {
$t_result = false;
} else {
$t_size = $p_size;
$t_default_avatar = config_get( 'show_avatar' );

if( http_is_protocol_https() ) {
$t_gravatar_domain = 'https://secure.gravatar.com/';
} else {
$t_gravatar_domain = 'http://www.gravatar.com/';
}
if( OFF === $t_default_avatar ) {
# Avatars are not used
return false;
}

$t_avatar_url = $t_gravatar_domain . 'avatar/' . md5( $t_email ) . '?d=identicon&r=G&s=' . $t_size;
# Default avatar is either one of Gravatar's options, or
# assumed to be an URL to a default avatar image
$t_default_avatar = urlencode( $t_default_avatar );
$t_rating = 'G';

$t_result = array(
$t_avatar_url,
$t_size,
$t_size,
);
$t_email_hash = md5( utf8_strtolower( trim( user_get_email( $p_user_id ) ) ) );

# Build Gravatar URL
if( http_is_protocol_https() ) {
$t_avatar_url = 'https://secure.gravatar.com/';
} else {
$t_avatar_url = 'http://www.gravatar.com/';
}
$t_avatar_url .= "avatar/$t_email_hash?d=$t_default_avatar&r=$t_rating&s=$p_size";

return $t_result;
return array( $t_avatar_url, $p_size, $p_size );
}

# --------------------
Expand Down
36 changes: 32 additions & 4 deletions docbook/administration_guide/en/configuration.sgml
View file
Expand Up @@ -896,10 +896,38 @@
<varlistentry>
<term>$g_show_avatar</term>
<listitem>
<para>Show user avatar (default OFF); the current implementation is based on http://www.gravatar.com,
users will need to register there with the same email address used in
this MantisBT installation to have their avatar shown.</para>
<note><para>Upon registration or avatar change, it takes some time for the updated gravatar images to show on sites.</para></note>
<para>Show the user's avatar
</para>
<para>The current implementation is based on
<ulink url="http://www.gravatar.com">Gravatar</ulink>;
Users will need to register there the same email address
used in this MantisBT installation to have their avatar
shown.
Please note: upon registration or avatar change, it may
take some time for the updated gravatar images to show
on sites
</para>
<para>The config can be either set to OFF (avatars disabled),
or to a string defining the default avatar to be used
when none is associated with the user's email.
Valid values are:
<itemizedlist>
<listitem><para>OFF (default)</para>
</listitem>
<listitem><para>One of Gravatar's defaults
(mm, identicon, monsterid, wavatar, retro);
refer to
<ulink url="http://en.gravatar.com/site/implement/images/">
Image Requests documentation
</ulink>
for further details.
</para></listitem>
<listitem><para>An URL to the default image to be used
(for example, "http:/path/to/unknown.jpg"
or "%path%images/no_avatar.png").
</para></listitem>
</itemizedlist>
</para>
</listitem>
</varlistentry>
<varlistentry>
Expand Down

4 comments on commit f710546

@atrol
Copy link
Member

@atrol atrol commented on f710546 Aug 15, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After upgrading old installations with setting $g_show_avatar = ON; will still work, but it's random behavior (seems that gravatar just ignores parameter ?d=1)

Did you check that mixed types (int|string) are working also with database configuration (adm_config_report.php)?
I had a short look and found only one other setting which uses mixed types ($g_limit_email_domain)

I prefer the simpler old approach to have to separate customization options for it.

@dregad
Copy link
Member Author

@dregad dregad commented on f710546 Aug 16, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @atrol

As usual, many thanks for your careful and thorough review :)

After upgrading old installations with setting $g_show_avatar = ON; will still work, but it's random behavior

I was actually thinking about the very same thing last night, and am in fact finalizing a patch to address exactly that, i.e. avoid regression when using legacy configuration (if ON, then Mantis will use the "identicon" default)

seems that gravatar just ignores parameter ?d=1

In my tests, behavior is not random, and d=1 is not ignored by Gravatar, it just uses the default "rotated G" image. Did you notice something different ?

Did you check that mixed types (int|string) are working also with database configuration (adm_config_report.php)?

Yes I did, and found no issues with that

I prefer the simpler old approach to have to separate customization options for it.

Both have their advantages - here we have one single config controlling everything in a rather simple and straightforward manner, which has the added benefit of reducing the number of global variables.

@dregad
Copy link
Member Author

@dregad dregad commented on f710546 Aug 16, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After upgrading old installations with setting $g_show_avatar = ON

Fixed - see fd20db7

@atrol
Copy link
Member

@atrol atrol commented on f710546 Aug 16, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my tests, behavior is not random, and d=1 is not ignored by Gravatar, it just uses the default "rotated G" image. Did you notice something different ?

With "ignored" I meant that
http://www.gravatar.com/avatar/00000000000000000000000000000000 and
http://www.gravatar.com/avatar/00000000000000000000000000000000?d=1
are producing the same result (the rotated G image)

With "random behavior" I meant that the behavior is not documented.
It's just the re-engineered reproducible way it works at the moment.
You can't expect that d=1 produces the rotated G.
Maybe Gravatar will change the implementation and d=1 will generate
an error bitmap in future versions.

Possibly the better word for it is accidental and not random?

Both have their advantages - here we have one single config controlling
everything in a rather simple and straightforward manner, which has the
added benefit of reducing the number of global variables.

I agree,
I wrote because most of the MantisBT code is not using this coding style and
probably because I am not experienced in untyped programming languages and dealing
with === and !== .

I am quite sure that a lot of developers that started with typed languages
have their problems with it.
I am a bit aware of type problems since the following fix
3cca927
That's why I don't like dynamic or mixed typing.

Fixed - see fd20db7

Thanks

Please sign in to comment.