From 612106b3a43f36652e53c14dede87a8450f2dd60 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 27 May 2011 11:49:23 +0100 Subject: [PATCH] MDL-20636 fix URL handling for previews to always use Moodle URL. Also, make the preview window big enough. #367 --- lib/questionlib.php | 53 ++++++++++++++++++++++++++--------------- mod/quiz/edit.php | 2 +- mod/quiz/editlib.php | 20 +++++++++++++--- mod/quiz/locallib.php | 34 ++++++++++++++++++-------- question/editlib.php | 18 +++++++++----- question/preview.php | 10 +++++--- question/previewlib.php | 17 +++++++------ 7 files changed, 105 insertions(+), 49 deletions(-) diff --git a/lib/questionlib.php b/lib/questionlib.php index c2cb777f9f1..9b47a4b3929 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -77,11 +77,6 @@ define("QUESTION_NUMANS_START", 3); */ define("QUESTION_NUMANS_ADD", 3); -/** - * The options used when popping up a question preview window in Javascript. - */ -define('QUESTION_PREVIEW_POPUP_OPTIONS', 'scrollbars=yes,resizable=yes,width=800,height=600'); - /** * Move one question type in a list of question types. If you try to move one element * off of the end, nothing will change. @@ -703,26 +698,46 @@ function question_move_category_to_context($categoryid, $oldcontextid, $newconte * be picked randomly. * @return string the URL. */ -function question_preview_url($questionid, $preferredbehaviour, $maxmark, $displayoptions, - $variant = null) { - $params = array( - 'id' => $questionid, - 'behaviour' => $preferredbehaviour, - 'maxmark' => $maxmark, - 'correctness' => $displayoptions->correctness, - 'marks' => $displayoptions->marks, - 'markdp' => $displayoptions->markdp, - 'feedback' => (bool) $displayoptions->feedback, - 'generalfeedback' => (bool) $displayoptions->generalfeedback, - 'rightanswer' => (bool) $displayoptions->rightanswer, - 'history' => (bool) $displayoptions->history - ); +function question_preview_url($questionid, $preferredbehaviour = null, + $maxmark = null, $displayoptions = null, $variant = null) { + + $params = array('id' => $questionid); + + if (!is_null($preferredbehaviour)) { + $params['behaviour'] = $preferredbehaviour; + } + + if (!is_null($maxmark)) { + $params['maxmark'] = $maxmark; + } + + if (!is_null($displayoptions)) { + $params['correctness'] = $displayoptions->correctness; + $params['marks'] = $displayoptions->marks; + $params['markdp'] = $displayoptions->markdp; + $params['feedback'] = (bool) $displayoptions->feedback; + $params['generalfeedback'] = (bool) $displayoptions->generalfeedback; + $params['rightanswer'] = (bool) $displayoptions->rightanswer; + $params['history'] = (bool) $displayoptions->history; + } + if ($variant) { $params['variant'] = $variant; } + return new moodle_url('/question/preview.php', $params); } +/** + * @return array that can be passed as $params to the {@link popup_action} constructor. + */ +function question_preview_popup_params() { + return array( + 'height' => 600, + 'width' => 800, + ); +} + /** * Given a list of ids, load the basic information about a set of questions from * the questions table. The $join and $extrafields arguments can be used together diff --git a/mod/quiz/edit.php b/mod/quiz/edit.php index 1241f8a8cdd..5207b4b87a9 100644 --- a/mod/quiz/edit.php +++ b/mod/quiz/edit.php @@ -153,7 +153,7 @@ if (!$course) { print_error('invalidcourseid', 'error'); } -$questionbank = new quiz_question_bank_view($contexts, $thispageurl, $course, $cm); +$questionbank = new quiz_question_bank_view($contexts, $thispageurl, $course, $cm, $quiz); $questionbank->set_quiz_has_attempts($quizhasattempts); // Log this visit. diff --git a/mod/quiz/editlib.php b/mod/quiz/editlib.php index c9360d2b1be..41c5f27738c 100644 --- a/mod/quiz/editlib.php +++ b/mod/quiz/editlib.php @@ -1079,6 +1079,21 @@ class question_bank_question_name_text_column extends question_bank_question_nam */ class quiz_question_bank_view extends question_bank_view { protected $quizhasattempts = false; + /** @var object the quiz settings. */ + protected $quiz = false; + + /** + * Constructor + * @param question_edit_contexts $contexts + * @param moodle_url $pageurl + * @param object $course course settings + * @param object $cm activity settings. + * @param object $quiz quiz settings. + */ + public function __construct($contexts, $pageurl, $course, $cm, $quiz) { + parent::__construct($contexts, $pageurl, $course, $cm); + $this->quiz = $quiz; + } protected function known_field_types() { $types = parent::known_field_types(); @@ -1105,9 +1120,8 @@ class quiz_question_bank_view extends question_bank_view { } } - public function preview_question_url($questionid) { - global $CFG; - return $CFG->wwwroot . '/question/preview.php?id=' . $questionid . $this->quizorcourseid; + public function preview_question_url($question) { + return quiz_question_preview_url($this->quiz, $question); } public function add_to_quiz_url($questionid) { diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 72aca38087b..9dfbc6bebfa 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -880,7 +880,27 @@ function quiz_question_edit_button($cmid, $question, $returnurl, $contentafteric } /** - * @param object $quiz the quiz + * @param object $quiz the quiz settings + * @param object $question the question + * @return moodle_url to preview this question with the options from this quiz. + */ +function quiz_question_preview_url($quiz, $question) { + // Get the appropriate display options. + $displayoptions = mod_quiz_display_options::make_from_quiz($quiz, + mod_quiz_display_options::DURING); + + $maxmark = null; + if (isset($question->maxmark)) { + $maxmark = $question->maxmark; + } + + // Work out the correcte preview URL. + return question_preview_url($question->id, $quiz->preferredbehaviour, + $maxmark, $displayoptions); +} + +/** + * @param object $quiz the quiz settings * @param object $question the question * @param bool $label if true, show the preview question label after the icon * @return the HTML for a preview question icon. @@ -891,13 +911,7 @@ function quiz_question_preview_button($quiz, $question, $label = false) { return ''; } - // Get the appropriate display options. - $displayoptions = mod_quiz_display_options::make_from_quiz($quiz, - mod_quiz_display_options::DURING); - - // Work out the correcte preview URL. - $url = question_preview_url($question->id, $quiz->preferredbehaviour, - $question->maxmark, $displayoptions); + $url = quiz_question_preview_url($quiz, $question); // Do we want a label? $strpreviewlabel = ''; @@ -909,8 +923,8 @@ function quiz_question_preview_button($quiz, $question, $label = false) { $strpreviewquestion = get_string('previewquestion', 'quiz'); $image = $OUTPUT->pix_icon('t/preview', $strpreviewquestion); - parse_str(QUESTION_PREVIEW_POPUP_OPTIONS, $options); - $action = new popup_action('click', $url, 'questionpreview', $options); + $action = new popup_action('click', $url, 'questionpreview', + question_preview_popup_params()); return $OUTPUT->action_link($url, $image, $action, array('title' => $strpreviewquestion)); } diff --git a/question/editlib.php b/question/editlib.php index d646644124a..9848e870a32 100644 --- a/question/editlib.php +++ b/question/editlib.php @@ -666,9 +666,9 @@ class question_bank_preview_action_column extends question_bank_action_column_ba // Build the icon. $image = $OUTPUT->pix_icon('t/preview', $this->strpreview); - $link = new moodle_url($this->qbank->preview_question_url($question->id)); - parse_str(QUESTION_PREVIEW_POPUP_OPTIONS, $options); - $action = new popup_action('click', $link, 'questionpreview', $options); + $link = $this->qbank->preview_question_url($question); + $action = new popup_action('click', $link, 'questionpreview', + question_preview_popup_params()); echo $OUTPUT->action_link($link, $image, $action, array('title' => $this->strpreview)); } @@ -844,6 +844,13 @@ class question_bank_view { protected $loadsql; protected $sqlparams; + /** + * Constructor + * @param question_edit_contexts $contexts + * @param moodle_url $pageurl + * @param object $course course settings + * @param object $cm (optional) activity settings. + */ public function __construct($contexts, $pageurl, $course, $cm = null) { global $CFG, $PAGE; @@ -1144,9 +1151,8 @@ class question_bank_view { return $this->editquestionurl->out(true, array('id' => $questionid, 'movecontext' => 1)); } - public function preview_question_url($questionid) { - global $CFG; - return $CFG->wwwroot . '/question/preview.php?id=' . $questionid . '&courseid=' . $this->course->id; + public function preview_question_url($question) { + return question_preview_url($question->id); } /** diff --git a/question/preview.php b/question/preview.php index cc84188af39..e6085a6a8b0 100644 --- a/question/preview.php +++ b/question/preview.php @@ -52,7 +52,8 @@ $options->set_from_request(); $PAGE->set_url(question_preview_url($id, $options->behaviour, $options->maxmark, $options)); // Get and validate exitsing preview, or start a new one. -$previewid = optional_param('previewid', 0, PARAM_ALPHANUM); +$previewid = optional_param('previewid', 0, PARAM_INT); + if ($previewid) { if (!isset($SESSION->question_previews[$previewid])) { print_error('notyourpreview', 'question'); @@ -104,6 +105,9 @@ $optionsform->set_data($options); if ($newoptions = $optionsform->get_submitted_data()) { // Set user preferences $options->save_user_preview_options($newoptions); + if (!isset($newoptions->variant)) { + $newoptions->variant = $options->variant; + } restart_preview($previewid, $question->id, $newoptions); } @@ -151,7 +155,7 @@ if (data_submitted() && confirm_sesskey()) { $scrollpos = optional_param('scrollpos', '', PARAM_RAW); if ($scrollpos !== '') { - $actionurl .= '&scrollpos=' . ((int) $scrollpos); + $actionurl->param('scrollpos', (int) $scrollpos); } redirect($actionurl); } @@ -181,7 +185,7 @@ $PAGE->set_heading($title); echo $OUTPUT->header(); // Start the question form. -echo '
', "\n"; echo '', "\n"; echo '', "\n"; diff --git a/question/previewlib.php b/question/previewlib.php index d3dcdc0330e..6e34333bc64 100644 --- a/question/previewlib.php +++ b/question/previewlib.php @@ -197,15 +197,15 @@ class question_preview_options extends question_display_options { * @return string URL fragment. Parameters needed in the URL when continuing * this preview. */ - public function get_query_string() { - $querystring = array(); + public function get_url_params() { + $params = array(); foreach ($this->get_field_types() as $field => $notused) { if ($field == 'behaviour' || $field == 'maxmark' || is_null($this->$field)) { continue; } - $querystring[] = $field . '=' . $this->$field; + $params[$field] = $this->$field; } - return implode('&', $querystring); + return $params; } } @@ -268,9 +268,12 @@ function question_preview_question_pluginfile($course, $context, $component, */ function question_preview_action_url($questionid, $qubaid, question_preview_options $options) { - global $CFG; - $url = $CFG->wwwroot . '/question/preview.php?id=' . $questionid . '&previewid=' . $qubaid; - return $url . '&' . $options->get_query_string(); + $params = array( + 'id' => $questionid, + 'previewid' => $qubaid, + ); + $params = array_merge($params, $options->get_url_params()); + return new moodle_url('/question/preview.php', $params); } /**