MDL-21198 going back to the original spacer syntax, image component improvements, plus some regression fixes

blog/external_blogs.php

@@ -72,14 +72,14 @@ if (!empty($blogs)) {
     $table->head = array(get_string('name'), get_string('url'), get_string('timefetched', 'blog'), get_string('valid', 'blog'), get_string('actions'));
 
     foreach ($blogs as $blog) {
-        $validicon = html_image::make($OUTPUT->pix_url('i/tick_green_big'));
-        $validicon->alt = get_string('feedisvalid', 'blog');
-        $validicon->title = get_string('feedisvalid', 'blog');
-
         if ($blog->failedlastsync) {
-            $validicon->src = $OUTPUT->pix_url('i/cross_red_big');
+            $validicon = html_image::make($OUTPUT->pix_url('i/cross_red_big'));
             $validicon->alt = get_string('feedisinvalid', 'blog');
             $validicon->title = get_string('feedisinvalid', 'blog');
+        } else {
+            $validicon = html_image::make($OUTPUT->pix_url('i/tick_green_big'));
+            $validicon->alt = get_string('feedisvalid', 'blog');
+            $validicon->title = get_string('feedisvalid', 'blog');
         }
 
         $editicon = new moodle_action_icon;

calendar/lib.php

@@ -545,10 +545,7 @@ function calendar_print_event($event, $showactions=true) {
     if (!empty($event->icon)) {
         echo $event->icon;
     } else {
-        $spacer = new html_image();
+        echo $OUTPUT->spacer(array('height'=>16, 'width'=>16, 'br'=>true)); // should be done with CSS instead
-        $spacer->height = 16;
-        $spacer->width = 16;
-        echo $OUTPUT->spacer($spacer) . '<br />';
     }
     echo '</td>';
     echo '<td class="topic">';

course/lib.php

@@ -1349,10 +1349,7 @@ function print_section($course, $section, $mods, $modnamesused, $absolute=false,
             }
 
             if ($mod->indent) {
-                $spacer = new html_image();
+                echo $OUTPUT->spacer(array('height'=>12, 'width'=>(20 * $mod->indent))); // should be done with CSS instead
-                $spacer->height = 12;
-                $spacer->width = 20 * $mod->indent;
-                echo $OUTPUT->spacer($spacer);
             }
 
             $extra = '';
@@ -1968,10 +1965,8 @@ function print_category_info($category, $depth, $showcourses = false) {
             $indent = $depth*30;
             $rows = count($courses) + 1;
             echo '<td class="category indentation" rowspan="'.$rows.'" valign="top">';
-            $spacer = new html_image();
+
-            $spacer->height = 10;
+            echo $OUTPUT->spacer(array('height'=>10, 'width'=>$indent, 'br'=>true)); // should be done with CSS instead
-            $spacer->width = $indent;
-            echo $OUTPUT->spacer($spacer) . '<br />';
             echo '</td>';
         }
 
@@ -2024,10 +2019,7 @@ function print_category_info($category, $depth, $showcourses = false) {
         if ($depth) {
             $indent = $depth*20;
             echo '<td class="category indentation" valign="top">';
-            $spacer = new html_image();
+            echo $OUTPUT->spacer(array('height'=>10, 'width'=>$indent, 'br'=>true)); // should be done with CSS instead
-            $spacer->height = 10;
-            $spacer->width = $indent;
-            echo $OUTPUT->spacer($spacer) . '<br />';
             echo '</td>';
         }
 

course/recent.php

@@ -225,9 +225,7 @@ if (!empty($activities)) {
         if (($activity->type == 'section') && ($param->sortby == 'default')) {
             if ($inbox) {
                 echo $OUTPUT->box_end();
-                $spacer = new html_image();
+                echo $OUTPUT->spacer(array('height'=>30, 'br'=>true)); // should be done with CSS instead
-                $spacer->height = 30;
-                echo $OUTPUT->spacer($spacer) . '<br />';
             }
             echo $OUTPUT->box_start();
             echo "<h2>$activity->name</h2>";

course/search.php

@@ -239,10 +239,7 @@
                 $course->summary .= $displaylist[$course->category];
                 $course->summary .= "</a></p>";
                 print_course($course, $search);
-                $spacer = new html_image();
+                echo $OUTPUT->spacer(array('height'=>5, 'width'=>5, 'br'=>true)); // should be done with CSS instead
-                $spacer->height = 5;
-                $spacer->width = 5;
-                echo $OUTPUT->spacer($spacer) . '<br />';
             }
         } else {
         /// Show editing UI.

lib/adminlib.php

@@ -6025,22 +6025,21 @@ class admin_setting_managerepository extends admin_setting {
 
             // display up/down link
             $updown = '';
-            $spacer = new html_image();
+            $spacer = $OUTPUT->spacer(array('height'=>15, 'width'=>15)); // should be done with CSS instead
-            $spacer->height = 15;
+
-            $spacer->width = 15;
             if ($updowncount > 1) {
                 $updown .= "<a href=\"$this->baseurl&amp;move=up&amp;type=".$i->get_typename()."\">";
                 $updown .= "<img src=\"" . $OUTPUT->pix_url('t/up') . "\" alt=\"up\" /></a>&nbsp;";
             }
             else {
-                $updown .= $OUTPUT->spacer($spacer);
+                $updown .= $spacer;
             }
             if ($updowncount < count($instances)) {
                 $updown .= "<a href=\"$this->baseurl&amp;move=down&amp;type=".$i->get_typename()."\">";
                 $updown .= "<img src=\"" . $OUTPUT->pix_url('t/down') . "\" alt=\"down\" /></a>";
             }
             else {
-                $updown .= $OUTPUT->spacer($spacer);
+                $updown .= $spacer;
             }
 
             $updowncount++;

lib/deprecatedlib.php

@@ -2613,15 +2613,7 @@ function print_spacer($height=1, $width=1, $br=true, $return=false) {
 
     debugging('print_spacer() has been deprecated. Please change your code to use $OUTPUT->spacer().');
 
-    $spacer = new html_image();
+    $output = $OUTPUT->spacer(array('height'=>$height, 'width'=>$width, 'br'=>$br));
-    $spacer->height = $height;
-    $spacer->width = $width;
-
-    $output = $OUTPUT->spacer($spacer);
-
-    if ($br) {
-        $output .= '<br />';
-    }
 
     if ($return) {
         return $output;

lib/outputcomponents.php

@@ -1357,6 +1357,14 @@ class html_image extends labelled_html_component {
      * @var string $src The path to the image being used
      */
     public $src;
+    /**
+     * @var int $width of image
+     */
+    public $width;
+    /**
+     * @var int $height of image
+     */
+    public $height;
 
     /**
      * @see lib/html_component#prepare()
@@ -1367,7 +1375,7 @@ class html_image extends labelled_html_component {
             throw new coding_exception('html_image requires a $src value (moodle_url).');
         }
 
-        $this->add_class('image'); //TODO. remove this like somehow
+        // no general class here, use custom class instead or img element directly in css selectors
         parent::prepare($output, $page, $target);
     }
 

lib/outputrenderers.php

@@ -204,6 +204,23 @@ class renderer_base {
         }
         return $output;
     }
+
+    /**
+     * Helper function for applying of html_component options
+     * @param html_component $component
+     * @param array $options
+     * @return void
+     */
+    protected function apply_component_options(html_component $component, array $options = null) {
+        $options = (array)$options;
+        foreach ($options as $key => $value) {
+            if ($key === 'class' or $key === 'classes') {
+                $component->add_classes($value);
+            } else if (array_key_exists($key, $component)) {
+                $component->$key = $value;
+            }
+        }
+    }
 }
 
 
@@ -1123,21 +1140,25 @@ class core_renderer extends renderer_base {
     /**
      * Creates and returns a spacer image with optional line break.
      *
-     * @param html_image $image Subclass of html_component
+     * @param array $options id, alt, width=1, height=1, etc.
-     *
+     *              special options br=false (break after spacer)
      * @return string HTML fragment
      */
-    public function spacer($image) {
+    public function spacer(array $options = null) {
-        $image = clone($image);
+        $options = (array)$options;
-
+        if (empty($options['width'])) {
-        if (empty($image->src)) {
+            $options['width'] = 1;
-            $image->src = $this->pix_url('spacer')->out(false, array(), false);
         }
+        if (empty($options['height'])) {
+            $options['height'] = 1;
+        }
+        $options['class'] = 'spacer';
 
-        $image->prepare($this, $this->page, $this->target);
+        $output = $this->image($this->pix_url('spacer'), $options);
-        $image->add_class('spacer');
 
-        $output = $this->image($image);
+        if (!empty($options['br'])) {
+            $output .= '<br />';
+        }
 
         return $output;
     }
@@ -1145,16 +1166,31 @@ class core_renderer extends renderer_base {
     /**
      * Creates and returns an image.
      *
-     * @param html_image $image Subclass of html_component
+     * @param html_image|moodle_url|string $image_or_url image or url of the image
+     * @param array $options image attributes such as title, id, alt, widht, height
      *
      * @return string HTML fragment
      */
-    public function image($image) {
+    public function image($image_or_url, array $options = null) {
-        if ($image === false) {
+        if ($image_or_url === false) {
             return false;
+
+        } else if ($image_or_url instanceof html_image) {
+            $image = clone($image_or_url);
+
+        } else {
+            if ($image_or_url instanceof moodle_url) {
+                $image = new html_image();
+                $image->src = clone($image_or_url);
+            } else {
+                // must be a string
+                $image = new html_image();
+                $image->src = new moodle_url($image_or_url);
+            }
+
+            $this->apply_component_options($image, $options);
         }
 
-        $image = clone($image);
         $image->prepare($this, $this->page, $this->target);
 
         $this->prepare_event_handlers($image);
@@ -1166,9 +1202,15 @@ class core_renderer extends renderer_base {
                             'title' => $image->title,
                             'id' => $image->id);
 
-        if (!empty($image->height) || !empty($image->width)) {
+        // do not use prepare_legacy_width_and_height() here,
-            $attributes['style'] .= $this->prepare_legacy_width_and_height($image);
+        // xhtml strict allows width&height and inline styles break theming too!
+        if (!empty($image->height)) {
+            $attributes['height'] = $image->height;
         }
+        if (!empty($image->width)) {
+            $attributes['width'] = $image->width;
+        }
+
         return $this->output_empty_tag('img', $attributes);
     }
 
@@ -1189,7 +1231,7 @@ class core_renderer extends renderer_base {
      * $OUTPUT->user_picture($userpic);
      * </pre>
      *
-     * @param object $user_or_userpicture Object with at least fields id, picture, imagealt, firstname, lastname
+     * @param object|user_picture $user_or_userpicture Object with at least fields id, picture, imagealt, firstname, lastname
      *     If any of these are missing, the database is queried. Avoid this
      *     if at all possible, particularly for reports. It is very bad for performance.
      * @param array $options associative array with user picture options, used only if not a user_picture object,
@@ -1199,6 +1241,7 @@ class core_renderer extends renderer_base {
      *     - link=true (make image clickable - the link leads to user profile)
      *     - popup=false (open in popup)
      *     - alttext=true (add image alt attribute)
+     *     - etc.
      * @return string HTML fragment
      */
     public function user_picture(stdClass $user_or_userpicture, array $options = null) {
@@ -1210,13 +1253,7 @@ class core_renderer extends renderer_base {
         } else {
             $userpic = new user_picture();
             $userpic->user = $user_or_userpicture;
-            $options = (array)$options;
+            $this->apply_component_options($userpic, $options);
-            $allowed = array('courseid', 'size', 'link', 'popup', 'alttext');
-            foreach ($allowed as $key) {
-                if (array_key_exists($key, $options)) {
-                    $userpic->$key = $options[$key];
-                }
-            }
         }
 
         $userpic->prepare($this, $this->page, $this->target);

mod/hotpot/report/default.php

@@ -352,10 +352,7 @@ class hotpot_default_report {
             $this->print_html_finish($table);
 
             if (($i+1)<$count) {
-                $spacer = new html_image();
+                echo $OUTPUT->spacer(array('height'=>30, 'width'=>10, 'br'=>true)); // should be done with CSS instead
-                $spacer->height = 30;
-                $spacer->width = 10;
-                echo $OUTPUT->spacer($spacer) . '<br />';
             }
         }
     }

mod/hotpot/review.php

@@ -150,10 +150,7 @@ function hotpot_print_review_buttons(&$course, &$hotpot, &$attempt, $context) {
     }
     print "</td>\n</tr>\n";
     print '<tr><td colspan="'.$colspan.'">';
-    $spacer = new html_image();
+    echo $OUTPUT->spacer(array('height'=>4, 'width'=>1)); // should be done with CSS instead
-    $spacer->height = 4;
-    $spacer->width = 1;
-    echo $OUTPUT->spacer($spacer);
     print "</td></tr>\n";
     print "</table>\n";
 }

mod/survey/report.php

@@ -167,10 +167,7 @@
     }
     echo $OUTPUT->box_end();
 
-    $spacer = new html_image();
+    echo $OUTPUT->spacer(array('height'=>30, 'width'=>30, 'br'=>true)); // should be done with CSS instead
-    $spacer->height = 30;
-    $spacer->width = 30;
-    echo $OUTPUT->spacer($spacer) . '<br />';
 
 
 /// Print the menu across the top
@@ -307,10 +304,7 @@
 
                     echo $OUTPUT->table($table);
 
-                    $spacer = new html_image();
+                    echo $OUTPUT->spacer(array('height'=>30)); // should be done with CSS instead
-                    $spacer->height = 30;
-                    $spacer->width = 1;
-                    echo $OUTPUT->spacer($spacer);
                 }
             }
         }

mod/survey/view.php

@@ -103,9 +103,7 @@
         } else {
 
             echo $OUTPUT->box(format_module_intro('survey', $survey, $cm->id), 'generalbox', 'intro');
-            $spacer = new html_image();
+            echo $OUTPUT->spacer(array('height'=>30, 'width'=>1, 'br'=>true)); // should be done with CSS instead
-            $spacer->height = 30;
-            echo $OUTPUT->spacer(clone($spacer)) . '<br />';
 
             $questions = $DB->get_records_list("survey_questions", "id", explode(',', $survey->questions));
             $questionorder = explode(",", $survey->questions);

search/stats.php

@@ -106,9 +106,7 @@ require_once($CFG->dirroot.'/search/lib.php');
         }
 
         echo $OUTPUT->table($admin_table);
-        $spacer = new html_image();
+        echo $OUTPUT->spacer(array('height'=>20, 'br'=>true)); // should be done with CSS instead
-        $spacer->height = 20;
-        echo $OUTPUT->spacer(clone($spacer)) . '<br />';
         echo $OUTPUT->heading($solutionsstr);
 
         unset($admin_table->data);