diff --git a/question/type/gapselect/backup/moodle2/backup_qtype_gapselect_plugin.class.php b/question/type/gapselect/backup/moodle2/backup_qtype_gapselect_plugin.class.php index 76a0615546a..9456d6b4f34 100644 --- a/question/type/gapselect/backup/moodle2/backup_qtype_gapselect_plugin.class.php +++ b/question/type/gapselect/backup/moodle2/backup_qtype_gapselect_plugin.class.php @@ -15,8 +15,7 @@ // along with Moodle. If not, see . /** - * @package moodlecore - * @subpackage backup-moodle2 + * @package qtype_gapselect * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -26,7 +25,7 @@ defined('MOODLE_INTERNAL') || die(); /** - * Provides the information to backup gapselect questions + * Provides the information to backup gapselect questions. * * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -34,37 +33,37 @@ defined('MOODLE_INTERNAL') || die(); class backup_qtype_gapselect_plugin extends backup_qtype_plugin { /** - * Returns the qtype information to attach to question element + * Returns the qtype information to attach to question element. */ protected function define_question_plugin_structure() { - // Define the virtual plugin element with the condition to fulfill + // Define the virtual plugin element with the condition to fulfill. $plugin = $this->get_plugin_element(null, '../../qtype', 'gapselect'); - // Create one standard named plugin element (the visible container) + // Create one standard named plugin element (the visible container). $pluginwrapper = new backup_nested_element($this->get_recommended_name()); - // connect the visible container ASAP + // Connect the visible container ASAP. $plugin->add_child($pluginwrapper); // This qtype uses standard question_answers, add them here - // to the tree before any other information that will use them + // to the tree before any other information that will use them. $this->add_question_question_answers($pluginwrapper); - // Now create the qtype own structures + // Now create the qtype own structures. $gapselect = new backup_nested_element('gapselect', array('id'), array( 'shuffleanswers', 'correctfeedback', 'correctfeedbackformat', 'partiallycorrectfeedback', 'partiallycorrectfeedbackformat', 'incorrectfeedback', 'incorrectfeedbackformat', 'shownumcorrect')); - // Now the own qtype tree + // Now the own qtype tree. $pluginwrapper->add_child($gapselect); - // set source to populate the data + // Set source to populate the data. $gapselect->set_source_table('question_gapselect', array('questionid' => backup::VAR_PARENTID)); - // don't need to annotate ids nor files + // Don't need to annotate ids or files. return $plugin; } diff --git a/question/type/gapselect/backup/moodle2/restore_qtype_gapselect_plugin.class.php b/question/type/gapselect/backup/moodle2/restore_qtype_gapselect_plugin.class.php index 55f7c96ff40..bed2368112d 100644 --- a/question/type/gapselect/backup/moodle2/restore_qtype_gapselect_plugin.class.php +++ b/question/type/gapselect/backup/moodle2/restore_qtype_gapselect_plugin.class.php @@ -15,8 +15,7 @@ // along with Moodle. If not, see . /** - * @package moodlecore - * @subpackage backup-moodle2 + * @package qtype_gapselect * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -26,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); /** - * restore plugin class that provides the necessary information - * needed to restore one gapselect qtype plugin + * Restore plugin class that provides the necessary information + * needed to restore one gapselect qtype plugin. * * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -35,22 +34,22 @@ defined('MOODLE_INTERNAL') || die(); class restore_qtype_gapselect_plugin extends restore_qtype_plugin { /** - * Returns the paths to be handled by the plugin at question level + * Returns the paths to be handled by the plugin at question level. */ protected function define_question_plugin_structure() { $paths = array(); - // This qtype uses question_answers, add them + // This qtype uses question_answers, add them. $this->add_question_question_answers($paths); - // Add own qtype stuff + // Add own qtype stuff. $elename = 'gapselect'; - // we used get_recommended_name() so this works + // We used get_recommended_name() so this works. $elepath = $this->get_pathfor('/gapselect'); $paths[] = new restore_path_element($elename, $elepath); - return $paths; // And we return the interesting paths + return $paths; // And we return the interesting paths. } /** @@ -62,18 +61,18 @@ class restore_qtype_gapselect_plugin extends restore_qtype_plugin { $data = (object)$data; $oldid = $data->id; - // Detect if the question is created or mapped + // Detect if the question is created or mapped. $oldquestionid = $this->get_old_parentid('question'); $newquestionid = $this->get_new_parentid('question'); $questioncreated = $this->get_mappingid('question_created', $oldquestionid) ? true : false; - // If the question has been created by restore, we need to create its question_gapselect too + // If the question has been created by restore, we need to create its question_gapselect too. if ($questioncreated) { - // Adjust some columns + // Adjust some columns. $data->questionid = $newquestionid; - // Insert record + // Insert record. $newitemid = $DB->insert_record('question_gapselect', $data); - // Create mapping (needed for decoding links) + // Create mapping (needed for decoding links). $this->set_mapping('question_gapselect', $oldid, $newitemid); } } diff --git a/question/type/gapselect/edit_form_base.php b/question/type/gapselect/edit_form_base.php index c9cc714ea58..fae6dfdd2e0 100644 --- a/question/type/gapselect/edit_form_base.php +++ b/question/type/gapselect/edit_form_base.php @@ -47,49 +47,59 @@ class qtype_gapselect_edit_form_base extends question_edit_form { ); /** @var string regex to match HTML open tags. */ - private $htmltstarttagsandattributes = '/<\s*\w.*?>/'; + private $htmltstarttagsandattributes = '~<\s*\w+\b[^>]*>~'; /** @var string regex to match HTML close tags or br. */ - private $htmltclosetags = '~<\s*/\s*\w\s*.*?>|<\s*br\s*>~'; + private $htmltclosetags = '~<\s*/\s*\w+\b[^>]*>~'; /** @var string regex to select text like [[cat]] (including the square brackets). */ private $squarebracketsregex = '/\[\[[^]]*?\]\]/'; - private function get_html_tags($text) { - $textarray = array(); + /** + * Vaidate some input to make sure it does not contain any tags other than + * $this->allowedhtmltags. + * @param unknown_type $text the input to validate. + * @return string any validation errors. + */ + protected function get_illegal_tag_error($text) { + // Remove legal tags. + $strippedtext = $text; foreach ($this->allowedhtmltags as $htmltag) { - $tagpair = "/<\s*\/?\s*$htmltag\s*.*?>/"; - preg_match_all($tagpair, $text, $textarray); - if ($textarray[0]) { - return $textarray[0]; - } + $tagpair = "~<\s*/?\s*$htmltag\b\s*[^>]*>~"; + $strippedtext = preg_replace($tagpair, '', $strippedtext); } - preg_match_all($this->htmltstarttagsandattributes, $text, $textarray); + $textarray = array(); + preg_match_all($this->htmltstarttagsandattributes, $strippedtext, $textarray); if ($textarray[0]) { - $tag = htmlspecialchars($textarray[0][0]); - $allowedtaglist = $this->get_list_of_printable_allowed_tags($this->allowedhtmltags); - return $tag . ' is not allowed (only ' . $allowedtaglist . - ' and corresponsing closing tags are allowed)'; + return $this->allowed_tags_message($textarray[0][0]); } - preg_match_all($this->htmltclosetags, $text, $textarray); + preg_match_all($this->htmltclosetags, $strippedtext, $textarray); if ($textarray[0]) { - $tag = htmlspecialchars($textarray[0][0]); - $allowedtaglist=$this->get_list_of_printable_allowed_tags($this->allowedhtmltags); - return $tag . ' is not allowed (only ' . $allowedtaglist . - ' and corresponsing closing tags are allowed)'; + return $this->allowed_tags_message($textarray[0][0]); } - return false; + return ''; + } + + private function allowed_tags_message($badtag) { + $a = new stdClass(); + $a->tag = htmlspecialchars($badtag); + $a->allowed = $this->get_list_of_printable_allowed_tags($this->allowedhtmltags); + if ($a->allowed) { + return get_string('tagsnotallowed', 'qtype_gapselect', $a); + } else { + return get_string('tagsnotallowedatall', 'qtype_gapselect', $a); + } } private function get_list_of_printable_allowed_tags($allowedhtmltags) { - $allowedtaglist = null; + $allowedtaglist = array(); foreach ($allowedhtmltags as $htmltag) { - $allowedtaglist .= htmlspecialchars('<'.$htmltag.'>') . ', '; + $allowedtaglist[] = htmlspecialchars('<' . $htmltag . '>'); } - return $allowedtaglist; + return implode(', ', $allowedtaglist); } /** @@ -99,7 +109,7 @@ class qtype_gapselect_edit_form_base extends question_edit_form { protected function definition_inner($mform) { global $CFG; - //add the answer (choice) fields to the form + // Add the answer (choice) fields to the form. $this->definition_answer_choice($mform); $this->add_combined_feedback_fields(true); @@ -192,7 +202,7 @@ class qtype_gapselect_edit_form_base extends question_edit_form { $questiontext = $data['questiontext']; $choices = $data['choices']; - //check the whether the slots are valid + // Check the whether the slots are valid. $errorsinquestiontext = $this->validate_slots($questiontext['text'], $choices); if ($errorsinquestiontext) { $errors['questiontext'] = $errorsinquestiontext; @@ -200,13 +210,10 @@ class qtype_gapselect_edit_form_base extends question_edit_form { foreach ($choices as $key => $choice) { $answer = $choice['answer']; - //check whether the html-tags are allowed tags - $validtags = $this->get_html_tags($answer); - if (is_array($validtags)) { - continue; - } - if ($validtags) { - $errors['choices['.$key.']'] = $validtags; + // Check whether the html-tags are allowed tags. + $tagerror = $this->get_illegal_tag_error($answer); + if ($tagerror) { + $errors['choices['.$key.']'] = $tagerror; } } return $errors; @@ -257,4 +264,4 @@ class qtype_gapselect_edit_form_base extends question_edit_form { public function qtype() { return ''; } -} \ No newline at end of file +} diff --git a/question/type/gapselect/lang/en/qtype_gapselect.php b/question/type/gapselect/lang/en/qtype_gapselect.php index 66ff0efff39..0369c706e3c 100644 --- a/question/type/gapselect/lang/en/qtype_gapselect.php +++ b/question/type/gapselect/lang/en/qtype_gapselect.php @@ -40,3 +40,5 @@ $string['pluginnameadding'] = 'Adding a select missing words question'; $string['pluginnameediting'] = 'Editing a select missing words question'; $string['pluginnamesummary'] = 'Missing words in some text are filled in using dropdown menus.'; $string['shuffle'] = 'Shuffle'; +$string['tagsnotallowed'] = '{$a->tag} is not allowed. (Only {$a->allowed} are permitted.)'; +$string['tagsnotallowedatall'] = '{$a->tag} is not allowed. (No HTML is allowed here.)'; diff --git a/question/type/gapselect/lang/fr/qtype_gapselect.php b/question/type/gapselect/lang/fr/qtype_gapselect.php deleted file mode 100644 index 7ecb6d6d0ee..00000000000 --- a/question/type/gapselect/lang/fr/qtype_gapselect.php +++ /dev/null @@ -1,42 +0,0 @@ -. - -/** - * Language strings for the gap-select question type. - * - * @package qtype_gapselect - * @copyright 2011 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -$string['addmorechoiceblanks'] = 'Vides pour {no} choix supplémentaires'; -$string['answer'] = 'Réponse'; -$string['choices'] = 'Choix'; -$string['choicex'] = 'Choix {no}'; -$string['correctansweris'] = 'La réponse correcte est : {$a}'; -$string['errorblankchoice'] = 'Veuillez sélectionner un choix : Le choix {$a} est vide.'; -$string['errormissingchoice'] = 'Veuillez sélectionner le texte de la question: {$a} n\'a pas été trouvé parmi les choix possibles! Seuls les numéros de choix existants peuvent être utilisés dans les espaces réservés.'; -$string['errornoslots'] = 'Le texte de la question doit contenir des espaces reservés du type [[1]] pour définir l\'emplacement des mots qui seront choisis.'; -$string['errorquestiontextblank'] = 'Vous devez entrer le texte de la question.'; -$string['group'] = 'Groupe'; -$string['pleaseputananswerineachbox'] = 'Veuillez entrer une réponse dans chaque boîte.'; -$string['pluginname'] = 'Sélectionnez les mots manquants'; -$string['pluginname_help'] = 'Entrez une question du type "Le [[1]] saute par-dessus le [[2]]", puis entrez les différents mots possibles pour les intervalles 1 et 2.'; -$string['pluginname_link'] = 'question/type/gapselect'; -$string['pluginnameadding'] = 'Ajouter une question permettant de sélectionner le mot manquant'; -$string['pluginnameediting'] = 'Editer une question permettant de sélectionner le mot manquant'; -$string['pluginnamesummary'] = 'Les mots manquants des textes sont comblés en utilisant des listes déroulantes.'; -$string['shuffle'] = 'Mélanger'; diff --git a/question/type/gapselect/lang/ru/qtype_gapselect.php b/question/type/gapselect/lang/ru/qtype_gapselect.php deleted file mode 100644 index 0e337abba29..00000000000 --- a/question/type/gapselect/lang/ru/qtype_gapselect.php +++ /dev/null @@ -1,42 +0,0 @@ -. - -/** - * Local language pack from http://www.moodle.vsu.ru - * - * @package qtype - * @subpackage gapselect - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -$string['addmorechoiceblanks'] = 'Шаблоны для ещё {no} вариантов'; -$string['answer'] = 'Ответ'; -$string['choices'] = 'Варианты выбора'; -$string['choicex'] = 'Вариант {no}'; -$string['correctansweris'] = 'Верный ответ: {$a}'; -$string['errorblankchoice'] = 'Пожалуйста, проверьте варианты выбора: вариант выбора {$a} пуст.'; -$string['errormissingchoice'] = 'Пожалуйста, проверьте текст вопроса: {$a} не найден среди вариантов выбора! Только номера ответов, которые существуют в вариантах выбора могут быть использованы как метки-заполнители.'; -$string['errornoslots'] = 'Текст вопроса должен содержать метки-заполнители, например [[1]], для обозначения местонахождения пропущенных слов.'; -$string['errorquestiontextblank'] = 'Вы должны ввести какой-либо текст.'; -$string['group'] = 'Группа'; -$string['pleaseputananswerineachbox'] = 'Пожалуйста, добавьте ответ в каждую ячейку'; -$string['pluginname'] = 'Выбрать пропущенные слова'; -$string['pluginnameadding'] = 'Добавить вопрос на выборку пропущенных слов'; -$string['pluginnameediting'] = 'Редактировать вопрос на выборку пропущенных слов'; -$string['pluginnamesummary'] = 'Пропущенные слова в тексте заполняются с помощью выпадающего меню.'; -$string['pluginname_help'] = 'Напишите текст, например "[[1]] перепрыгивает через [[2]]", затем введите возможные слова для меток 1 и 2 внизу'; -$string['shuffle'] = 'Перемешать'; diff --git a/question/type/gapselect/questionbase.php b/question/type/gapselect/questionbase.php index b561c273989..85794f03024 100644 --- a/question/type/gapselect/questionbase.php +++ b/question/type/gapselect/questionbase.php @@ -18,8 +18,7 @@ * Definition class for embedded element in question text question. Parent of * gap-select, drag and drop and possibly others. * - * @package qtype - * @subpackage gapselect + * @package qtype_gapselect * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -30,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); /** * Represents embedded element in question text question. Parent of drag and drop and select from - * drop down list and ?others? + * drop down list and others. * * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later diff --git a/question/type/gapselect/questiontypebase.php b/question/type/gapselect/questiontypebase.php index ec13a330b4c..a5a192a2826 100644 --- a/question/type/gapselect/questiontypebase.php +++ b/question/type/gapselect/questiontypebase.php @@ -56,7 +56,7 @@ abstract class qtype_gapselect_base extends question_type { $oldanswers = $DB->get_records('question_answers', array('question' => $question->id), 'id ASC'); - // Insert all the new answers + // Insert all the new answers. foreach ($question->choices as $key => $choice) { if (trim($choice['answer']) == '') { @@ -82,7 +82,7 @@ abstract class qtype_gapselect_base extends question_type { } } - // Delete old answer records + // Delete old answer records. foreach ($oldanswers as $oa) { $DB->delete_records('question_answers', array('id' => $oa->id)); } @@ -259,7 +259,7 @@ abstract class qtype_gapselect_base extends question_type { return false; } - //get the slots + // Get the slots. $slots = $this->getEmbeddedTextArray($question); if (!$slots) { @@ -269,7 +269,7 @@ abstract class qtype_gapselect_base extends question_type { $output = array(); foreach ($slots as $slot) { - $output[] = substr($slot, 2, strlen($slot) - 4); //2 is for '[[' and 4 is for '[[]]'. + $output[] = substr($slot, 2, strlen($slot) - 4); // 2 is for '[[' and 4 is for '[[]]'. } return $output; } @@ -282,7 +282,7 @@ abstract class qtype_gapselect_base extends question_type { } } - // Shuffle answers within this group + // Shuffle answers within this group. if ($question->options->shuffleanswers == 1) { shuffle($goupofanswers); } diff --git a/question/type/gapselect/tests/edit_form_test.php b/question/type/gapselect/tests/edit_form_test.php new file mode 100644 index 00000000000..19b3f3dd299 --- /dev/null +++ b/question/type/gapselect/tests/edit_form_test.php @@ -0,0 +1,125 @@ +. + +/** + * Unit tests for the select missing words question definition class. + * + * @package qtype_gapselect + * @copyright 2012 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +defined('MOODLE_INTERNAL') || die(); +global $CFG; + +require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); +require_once($CFG->dirroot . '/question/type/edit_question_form.php'); +require_once($CFG->dirroot . '/question/type/gapselect/edit_form_base.php'); + + +/** + * Subclass of qtype_gapselect_edit_form_base that is easier to used in unit tests. + * + * @copyright 2012 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_gapselect_edit_form_base_testable extends qtype_gapselect_edit_form_base { + public function __construct() { + $syscontext = context_system::instance(); + $category = question_make_default_categories(array($syscontext)); + $fakequestion = new stdClass(); + $fakequestion->qtype = 'stack'; + $fakequestion->category = $category->id; + $fakequestion->questiontext = 'Test [[1]] question [[2]]'; + $fakequestion->options = new stdClass(); + $fakequestion->options->answers = array(); + $fakequestion->formoptions = new stdClass(); + $fakequestion->formoptions->movecontext = null; + $fakequestion->formoptions->repeatelements = true; + $fakequestion->inputs = null; + parent::__construct(new moodle_url('/'), $fakequestion, $category, + new question_edit_contexts($syscontext)); + } + + // Make this public so we can test it. + public function get_illegal_tag_error($text) { + return parent::get_illegal_tag_error($text); + } + + /** + * Set the list of allowed tags. + * @param array $allowed + */ + public function set_allowed_tags(array $allowed) { + $this->allowedhtmltags = $allowed; + } +} + + +/** + * Unit tests for Stack question editing form. + * + * @copyright 2012 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_gapselect_edit_form_test extends advanced_testcase { + + protected function get_form() { + $this->setAdminUser(); + $this->resetAfterTest(); + + return new qtype_gapselect_edit_form_base_testable(); + } + + public function test_get_illegal_tag_error() { + $form = $this->get_form(); + + $this->assertEquals('', $form->get_illegal_tag_error('frog')); + $this->assertEquals('', $form->get_illegal_tag_error('toad')); + + $a = new stdClass(); + $a->tag = '<ijk>'; + $a->allowed = '<sub>, <sup>, <b>, <i>, <em>, <strong>'; + $this->assertEquals(get_string('tagsnotallowed', 'qtype_gapselect', $a), $form->get_illegal_tag_error('')); + + $a->tag = '</cat>'; + $this->assertEquals(get_string('tagsnotallowed', 'qtype_gapselect', $a), $form->get_illegal_tag_error('')); + + $a->tag = '<br />'; + $this->assertEquals(get_string('tagsnotallowed', 'qtype_gapselect', $a), $form->get_illegal_tag_error('
')); + + $form->set_allowed_tags(array()); + + $this->assertEquals('', $form->get_illegal_tag_error('frog')); + + $a->tag = '<i>'; + $this->assertEquals(get_string('tagsnotallowedatall', 'qtype_gapselect', $a), + $form->get_illegal_tag_error('toad')); + + $a->tag = '<ijk>'; + $this->assertEquals(get_string('tagsnotallowedatall', 'qtype_gapselect', $a), + $form->get_illegal_tag_error('')); + + $a->tag = '</cat>'; + $this->assertEquals(get_string('tagsnotallowedatall', 'qtype_gapselect', $a), + $form->get_illegal_tag_error('')); + + $a->tag = '<i>'; + $this->assertEquals(get_string('tagsnotallowedatall', 'qtype_gapselect', $a), + $form->get_illegal_tag_error('
')); + } +}