Skip to content

Commit

Permalink
MDL-21198 improved/simplified $OUTPUT->user_picture() + regression fi…
Browse files Browse the repository at this point in the history 
…xes, performance fixes and detected performance problems when printing user avatars
  • Loading branch information
@skodak
skodak committed Dec 27, 2009
1 parent 3405956 commit 812dbaf
Show file tree
Hide file tree
Showing 57 changed files with 183 additions and 264 deletions.
2 changes: 1 addition & 1 deletion blocks/messages/block_messages.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function get_content() {
foreach ($users as $user) {
$timeago = format_time(time() - $user->lastaccess);
$this->content->text .= '<li class="listentry"><div class="user"><a href="'.$CFG->wwwroot.'/user/view.php?id='.$user->id.'&amp;course='.SITEID.'" title="'.$timeago.'">';
$this->content->text .= $OUTPUT->user_picture(moodle_user_picture::make($user, SITEID));
$this->content->text .= $OUTPUT->user_picture($user, array('courseid'=>SITEID)); //TODO: user might not have capability to view frontpage profile :-(
$this->content->text .= fullname($user).'</a></div>';
$this->content->text .= '<div class="message"><a href="'.$CFG->wwwroot.'/message/discussion.php?id='.$user->id.'" onclick="this.target=\'message_'.$user->id.'\'; return openpopup(\'/message/discussion.php?id='.$user->id.'\', \'message_'.$user->id.'\', \'menubar=0,location=0,scrollbars,status,resizable,width=400,height=500\', 0);"><img class="iconsmall" src="'.$OUTPUT->pix_url('t/message') . '" alt="" />&nbsp;'.$user->count.'</a>';
$this->content->text .= '</div></li>';
Expand Down
6 changes: 2 additions & 4 deletions blocks/online_users/block_online_users.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,14 @@ function get_content() {
foreach ($users as $user) {
$this->content->text .= '<li class="listentry">';
$timeago = format_time(time() - $user->lastaccess); //bruno to calculate correctly on frontpage
$userpic = moodle_user_picture::make($user, $this->page->course->id);
$userpic->size = 16;

if ($user->username == 'guest') {
$this->content->text .= '<div class="user">'.$OUTPUT->user_picture($userpic);
$this->content->text .= '<div class="user">'.$OUTPUT->user_picture($user, array('size'=>16));
$this->content->text .= get_string('guestuser').'</div>';

} else {
$this->content->text .= '<div class="user"><a href="'.$CFG->wwwroot.'/user/view.php?id='.$user->id.'&amp;course='.$this->page->course->id.'" title="'.$timeago.'">';
$this->content->text .= '<div class="user">'.$OUTPUT->user_picture($userpic);
$this->content->text .= '<div class="user">'.$OUTPUT->user_picture($user, array('size'=>16));
$this->content->text .= $user->fullname.'</a></div>';
}
if ($canshowicon and ($USER->id != $user->id) and $user->username != 'guest') { // Only when logged in and messaging active etc
Expand Down
2 changes: 1 addition & 1 deletion blog/locallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public function print_html($return=false) {

$picturecell = new html_table_cell();
$picturecell->add_classes('picture left');
$picturecell->text = $OUTPUT->user_picture(moodle_user_picture::make($user, SITEID));
$picturecell->text = $OUTPUT->user_picture($user);

$table->head[] = $picturecell;

Expand Down
2 changes: 1 addition & 1 deletion grade/report/grader/ajaxlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function get_studentrowhtml($user) {
// Student name and link
$user_pic = null;
if ($showuserimage) {
$user_pic = '<div class="userpic">' . $OUTPUT->user_picture(moodle_user_picture::make($user, $this->courseid)) . '</div>';
$user_pic = '<div class="userpic">' . $OUTPUT->user_picture($user) . '</div>';
}

$studentrowhtml .= '<tr class="r'.$this->rowcount++ . $row_classes[$this->rowcount % 2] . '">'
Expand Down
2 changes: 1 addition & 1 deletion grade/report/grader/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ public function get_left_rows() {
$usercell->add_action('click', 'yui_set_row');

if ($showuserimage) {
$usercell->text = $OUTPUT->container($OUTPUT->user_picture(moodle_user_picture::make($user, $this->courseid)), 'userpic');
$usercell->text = $OUTPUT->container($OUTPUT->user_picture($user), 'userpic');
}

$usercell->text .= $OUTPUT->link(html_link::make(new moodle_url($CFG->wwwroot.'/user/view.php', array('id' => $user->id, 'course' => $this->course->id)), fullname($user)));
Expand Down
10 changes: 2 additions & 8 deletions lib/commentlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,11 +396,7 @@ public function get_comments($page = '') {
$user->lastname = $c->lastname;
$user->imagealt = $c->imagealt;
$c->content = format_text($c->content, $c->format);
$userpic = moodle_user_picture::make($user, $this->course->id);
$userpic->link = true;
$userpic->size = 18;
$userpic->alttext = true;
$c->avatar = $OUTPUT->user_picture($userpic);
$c->avatar = $OUTPUT->user_picture($user, array('size'=>18));
if (($USER->id == $c->userid) || !empty($candelete)) {
$c->delete = true;
}
Expand Down Expand Up @@ -485,9 +481,7 @@ public function add($content, $format = FORMAT_MOODLE) {
$newcmt->time = userdate($now, get_string('strftimerecent', 'langconfig'));
$newcmt->username = fullname($USER);
$newcmt->content = format_text($newcmt->content);
$userpic = moodle_user_picture::make($USER, $this->course->id);
$userpic->size = 16;
$newcmt->avatar = $OUTPUT->user_picture($userpic);
$newcmt->avatar = $OUTPUT->user_picture($user, array('size'=>16));
return $newcmt;
} else {
throw new comment_exception('dbupdatefailed');
Expand Down
25 changes: 11 additions & 14 deletions lib/deprecatedlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2661,26 +2661,23 @@ function print_file_picture($path, $courseid=0, $height='', $width='', $link='',
* @return string|void String or nothing, depending on $return.
*/
function print_user_picture($user, $courseid, $picture=NULL, $size=0, $return=false, $link=true, $target='', $alttext=true) {
global $CFG, $DB, $OUTPUT;

debugging('print_user_picture() has been deprecated. Please change your code to use $OUTPUT->user_picture($user, $courseid).');
global $OUTPUT;

$userpic = new moodle_user_picture();
$userpic->user = $user;
$userpic->courseid = $courseid;
$userpic->size = $size;
$userpic->link = $link;
$userpic->alttext = $alttext;
debugging('print_user_picture() has been deprecated. Please change your code to use $OUTPUT->user_picture($user, array(\'courseid\'=>$courseid).');

if (!empty($picture)) {
$userpic->image->src = $picture;
if (!is_object($user)) {
$userid = $user;
$user = new object();
$user->id = $userid;
}

if (!empty($target)) {
$userpic->add_action(new popup_action('click', new moodle_url($target)));
if (empty($user->picture) and $picture) {
$user->picture = $picture;
}

$output = $OUTPUT->user_picture($userpic);
$options = array('size'=>$size, 'link'=>$link, 'alttext'=>$alttext, 'courseid'=>$courseid, 'popup'=>!empty($target));

$output = $OUTPUT->user_picture($user, $options);

if ($return) {
return $output;
Expand Down
126 changes: 49 additions & 77 deletions lib/outputcomponents.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@ public function prepare(renderer_base $output, moodle_page $page, $target) {
throw new coding_exception('html_image requires a $src value (moodle_url).');
}

$this->add_class('image');
$this->add_class('image'); //TODO. remove this like somehow
parent::prepare($output, $page, $target);
}

Expand Down Expand Up @@ -1817,43 +1817,38 @@ public function make($totalcount, $page, $perpage, $baseurl) {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @since Moodle 2.0
*/
class moodle_user_picture extends moodle_html_component {
class user_picture extends html_image {
/**
* @var mixed $user A userid or a user object with at least fields id, picture, imagealrt, firstname and lastname set.
* @var mixed $user A user object with at least fields id, picture, imagealt, firstname and lastname set.
*/
public $user;
/**
* @var int $courseid The course id. Used when constructing the link to the user's profile.
* @var int $courseid The course id. Used when constructing the link to the user's profile,
* page course id used if not specified.
*/
public $courseid;
/**
* @var html_image $image A custom image used as the user picture.
* @var bool $link add course profile link to image
*/
public $image;
/**
* @var mixed $url False: picture not enclosed in a link. True: default link. moodle_url: custom link.
*/
public $url;
public $link = true;
/**
* @var int $size Size in pixels. Special values are (true/1 = 100px) and (false/0 = 35px) for backward compatibility
*/
public $size;
public $size = 35;
/**
* @var boolean $alttext add non-blank alt-text to the image. (Default true, set to false for purely
* @var boolean $alttext add non-blank alt-text to the image.
* Default true, set to false when image alt just duplicates text in screenreaders.
*/
public $alttext = true;
/**
* @var boolean $popup Whether or not to open the link in a popup window
* @var boolean $popup Whether or not to open the link in a popup window.
*/
public $popup = false;

/**
* Constructor: sets up the other components in case they are needed
* @return void
* @var link to profile if link requested
*/
public function __construct() {
$this->image = new html_image();
}
public $url;

/**
* @see lib/moodle_html_component#prepare()
Expand All @@ -1863,55 +1858,52 @@ public function prepare(renderer_base $output, moodle_page $page, $target) {
global $CFG, $DB;

if (empty($this->user)) {
throw new coding_exception('A moodle_user_picture object must have a $user object before being rendered.');
throw new coding_exception('A user_picture object must have a $user object before being rendered.');
}

if (empty($this->courseid)) {
throw new coding_exception('A moodle_user_picture object must have a courseid value before being rendered.');
if (empty($this->user->id)) {
throw new coding_exception('User id missing in $user object.');
}

if (!($this->image instanceof html_image)) {
debugging('moodle_user_picture::image must be an instance of html_image', DEBUG_DEVELOPER);
if (empty($this->courseid)) {
$courseid = $page->course->id;
} else {
$courseid = $this->courseid;
}

// only touch the DB if we are missing data and complain loudly...
$needrec = false;
// only touch the DB if we are missing data...
if (is_object($this->user)) {
// Note - both picture and imagealt _can_ be empty
// what we are trying to see here is if they have been fetched
// from the DB. We should use isset() _except_ that some installs
// have those fields as nullable, and isset() will return false
// on null. The only safe thing is to ask array_key_exists()
// which works on objects. property_exists() isn't quite
// what we want here...
if (! (array_key_exists('picture', $this->user)
&& ($this->alttext && array_key_exists('imagealt', $this->user)
|| (isset($this->user->firstname) && isset($this->user->lastname)))) ) {
$needrec = true;
$this->user = $this->user->id;
}
} else {
if ($this->alttext) {
// we need firstname, lastname, imagealt, can't escape...

if (!array_key_exists('picture', $this->user)) {
$needrec = true;
debugging('Missing picture property in $user object, this is a performance problem that needs to be fixed by a developer.', DEBUG_DEVELOPER);
}
if ($this->alttext) {
if (!array_key_exists('firstname', $this->user) || !array_key_exists('lastname', $this->user) || !array_key_exists('imagealt', $this->user)) {
$needrec = true;
} else {
$userobj = new StdClass; // fake it to save DB traffic
$userobj->id = $this->user;
$userobj->picture = $this->image->src;
$this->user = clone($userobj);
unset($userobj);
debugging('Missing firstname, lastname or imagealt property in $user object, this is a performance problem that needs to be fixed by a developer.', DEBUG_DEVELOPER);
}
}

if ($needrec) {
$this->user = $DB->get_record('user', array('id' => $this->user), 'id,firstname,lastname,imagealt');
$this->user = $DB->get_record('user', array('id'=>$this->user->id), 'id, firstname, lastname, imagealt');
}

if ($this->url === true) {
$this->url = new moodle_url('/user/view.php', array('id' => $this->user->id, 'course' => $this->courseid));
if ($this->alttext) {
if (!empty($user->imagealt)) {
$this->alt = $user->imagealt;
} else {
$this->alt = get_string('pictureof', '', fullname($this->user));
}
} else {
$this->alt = HTML_ATTR_EMPTY;
}

if (!empty($this->url) && $this->popup) {
$this->add_action(new popup_action('click', $this->url));
if ($this->link) {
$this->url = new moodle_url('/user/view.php', array('id' => $this->user->id, 'course' => $courseid));
} else {
$this->url = null;
$this->popup = false;
}

if (empty($this->size)) {
Expand All @@ -1927,42 +1919,22 @@ public function prepare(renderer_base $output, moodle_page $page, $target) {
}

if (!empty($this->size)) {
$this->image->width = $this->size;
$this->image->height = $this->size;
$this->width = $this->size;
$this->height = $this->size;
}

$this->add_class('userpicture');

if (empty($this->image->src) && !empty($this->user->picture)) {
$this->image->src = $this->user->picture;
}

if (!empty($this->image->src)) {
if (!empty($this->user->picture)) {
require_once($CFG->libdir.'/filelib.php');
$this->image->src = new moodle_url(get_file_url($this->user->id.'/'.$file.'.jpg', null, 'user'));
$this->src = new moodle_url(get_file_url($this->user->id.'/'.$file.'.jpg', null, 'user'));
} else { // Print default user pictures (use theme version if available)
$this->add_class('defaultuserpic');
$this->image->src = $output->pix_url('u/' . $file);
}

if ($this->alttext) {
if (!empty($this->user->imagealt)) {
$this->image->alt = $this->user->imagealt;
} else {
// No alt text when there is nothing useful (accessibility)
$this->image->alt = HTML_ATTR_EMPTY;
}
$this->src = $output->pix_url('u/' . $file);
}

parent::prepare($output, $page, $target);
}

public static function make($user, $courseid) {
$userpic = new moodle_user_picture();
$userpic->user = $user;
$userpic->courseid = $courseid;
return $userpic;
}
}


Expand Down
Loading

0 comments on commit 812dbaf

Please sign in to comment.