MDL-24594 Fix some issues with the display of HTML choices.

The most significant issue is that the HTML editor alwasy wraps <p> tags round the input, but that is not appropriate for the choices. It is especially not appropriate because we want to display the choices in a <lable> for accessibility and usability reasons. In valid HTML label can only contain inline elemnts. Therefore, I introduced a make_html_inline method, with a minimal implementation. (It could be improved in future.)

Long term, I think the best option would be a new form field type, editorinline, or something like that. That would be a smaller version of TinyMCE that only lets you enter inline elements.

question/type/multichoice/question.php

@@ -124,6 +124,13 @@ abstract class qtype_multichoice_base extends question_graded_automatically {
                     $args, $forcedownload);
         }
     }
+
+    public function make_html_inline($html) {
+        $html = preg_replace('~\s*<p>\s*~', '', $html);
+        $html = preg_replace('~\s*</p>\s*~', '<br />', $html);
+        $html = preg_replace('~<br />$~', '', $html);
+        return $html;
+    }
 }
 
 

question/type/multichoice/questiontype.php

@@ -171,7 +171,7 @@ class qtype_multichoice extends question_type {
         }
         $this->initialise_combined_feedback($question, $questiondata, true);
 
-        $this->initialise_question_answers($question, $questiondata);
+        $this->initialise_question_answers($question, $questiondata, false);
     }
 
     public function delete_question($questionid, $contextid) {
@@ -194,8 +194,9 @@ class qtype_multichoice extends question_type {
             $responses = array();
 
             foreach ($questiondata->options->answers as $aid => $answer) {
-                $responses[$aid] = new question_possible_response($answer->answer,
+                $responses[$aid] = new question_possible_response(html_to_text(format_text(
-                        $answer->fraction);
+                        $answer->answer, $answer->answerformat, array('noclean' => true)),
+                        0, false), $answer->fraction);
             }
 
             $responses[null] = question_possible_response::no_response();
@@ -205,7 +206,9 @@ class qtype_multichoice extends question_type {
 
             foreach ($questiondata->options->answers as $aid => $answer) {
                 $parts[$aid] = array($aid =>
-                        new question_possible_response($answer->answer, $answer->fraction));
+                        new question_possible_response(html_to_text(format_text(
+                        $answer->answer, $answer->answerformat, array('noclean' => true)),
+                        0, false), $answer->fraction));
             }
 
             return $parts;

question/type/multichoice/renderer.php

@@ -94,8 +94,9 @@ abstract class qtype_multichoice_renderer_base extends qtype_with_combined_feedb
             $radiobuttons[] = $hidden . html_writer::empty_tag('input', $inputattributes) .
                     html_writer::tag('label',
                         $this->number_in_style($value, $question->answernumbering) .
-                        $question->format_text($ans->answer, $ans->answerformat,
+                        $question->make_html_inline($question->format_text(
-                                $qa, 'question', 'answer', $ansid),
+                                $ans->answer, $ans->answerformat,
+                                $qa, 'question', 'answer', $ansid)),
                     array('for' => $inputattributes['id']));
 
             // $options->suppresschoicefeedback is a hack specific to the
@@ -104,8 +105,9 @@ abstract class qtype_multichoice_renderer_base extends qtype_with_combined_feedb
             if ($options->feedback && empty($options->suppresschoicefeedback) &&
                     $isselected && trim($ans->feedback)) {
                 $feedback[] = html_writer::tag('div',
-                        $question->format_text($ans->feedback, $ans->feedbackformat,
+                        $question->make_html_inline($question->format_text(
-                                $qa, 'question', 'answerfeedback', $ansid),
+                                $ans->feedback, $ans->feedbackformat,
+                                $qa, 'question', 'answerfeedback', $ansid)),
                         array('class' => 'specificfeedback'));
             } else {
                 $feedback[] = '';

question/type/multichoice/simpletest/testquestion.php

@@ -135,6 +135,18 @@ class qtype_multichoice_single_question_test extends UnitTestCase {
                 $mc->id => question_classified_response::no_response(),
             ), $mc->classify_response(array()));
     }
+
+    public function test_make_html_inline() {
+        $mc = test_question_maker::make_a_multichoice_single_question();
+        $this->assertEqual('Frog', $mc->make_html_inline('<p>Frog</p>'));
+        $this->assertEqual('Frog<br />Toad', $mc->make_html_inline("<p>Frog</p>\n<p>Toad</p>"));
+        $this->assertEqual('<img src="http://example.com/pic.png" alt="Graph" />',
+                $mc->make_html_inline(
+                    '<p><img src="http://example.com/pic.png" alt="Graph" /></p>'));
+        $this->assertEqual("Frog<br />XXX <img src='http://example.com/pic.png' alt='Graph' />",
+                $mc->make_html_inline(" <p> Frog </p> \n\r
+                    <p> XXX <img src='http://example.com/pic.png' alt='Graph' /> </p> "));
+    }
 }
 
 

question/type/multichoice/simpletest/testquestiontype.php

@@ -54,8 +54,10 @@ class qtype_multichoice_test extends UnitTestCase {
         $q = new stdClass();
         $q->id = 1;
         $q->options->single = true;
-        $q->options->answers[1] = (object) array('answer' => 'frog', 'fraction' => 1);
+        $q->options->answers[1] = (object) array('answer' => 'frog',
-        $q->options->answers[2] = (object) array('answer' => 'toad', 'fraction' => 0);
+                'answerformat' => FORMAT_HTML, 'fraction' => 1);
+        $q->options->answers[2] = (object) array('answer' => 'toad',
+                'answerformat' => FORMAT_HTML, 'fraction' => 0);
 
         return $q;
     }

question/type/multichoice/styles.css

@@ -1,5 +1,4 @@
 .que.multichoice .answer .specificfeedback {
-    display: inline;
     padding: 0 0.7em;
     background: #FFF3BF;
 }

question/type/questionbase.php

@@ -298,9 +298,7 @@ abstract class question_definition {
      * @return string the equivalent plain text.
      */
     public function html_to_text($text, $format) {
-        $formatoptions = new stdClass();
+        return html_to_text(format_text($text, $format, array('noclean' => true)), 0, false);
-        $formatoptions->noclean = true;
-        return html_to_text(format_text($text, $format, $formatoptions), 0, false);
     }
 
     /** @return the result of applying {@link format_text()} to the question text. */

question/type/questiontypebase.php

@@ -751,8 +751,14 @@ class question_type {
      * Initialise question_definition::answers field.
      * @param question_definition $question the question_definition we are creating.
      * @param object $questiondata the question data loaded from the database.
+     * @param bool $forceplaintextanswers most qtypes assume that answers are
+     *      FORMAT_PLAIN, and dont use the answerformat DB column (it contains
+     *      the default 0 = FORMAT_MOODLE). Therefore, by default this method
+     *      ingores answerformat. Pass false here to use answerformat. For example
+     *      multichoice does this.
      */
-    protected function initialise_question_answers(question_definition $question, $questiondata) {
+    protected function initialise_question_answers(question_definition $question,
+            $questiondata, $forceplaintextanswers = true) {
         $question->answers = array();
         if (empty($questiondata->options->answers)) {
             return;
@@ -760,6 +766,9 @@ class question_type {
         foreach ($questiondata->options->answers as $a) {
             $question->answers[$a->id] = new question_answer($a->id, $a->answer,
                     $a->fraction, $a->feedback, $a->feedbackformat);
+            if (!$forceplaintextanswers) {
+                $question->answers[$a->id]->answerformat = $a->answerformat;
+            }
         }
     }