Merge branch 'MDL-32688-m23' of git://github.com/itamart/moodle into MOODLE_23_STABLE
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 17 Sep 2012 23:53:50 +0000 (01:53 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 17 Sep 2012 23:53:50 +0000 (01:53 +0200)
35 files changed:
blocks/rss_client/block_rss_client.php
lib/blocklib.php
lib/dml/moodle_database.php
lib/dml/mysqli_native_moodle_database.php
lib/dml/tests/dml_test.php
lib/dtl/database_exporter.php
lib/editor/tinymce/tiny_mce/3.5.1.1/plugins/moodlemedia/js/media.js
lib/editor/tinymce/tiny_mce/3.5.1.1/plugins/moodlemedia/preview.php
lib/filelib.php
lib/filestorage/file_storage.php
lib/javascript-static.js
lib/moodlelib.php
lib/outputrenderers.php
lib/pluginlib.php
mod/assign/backup/moodle2/backup_assign_stepslib.php
mod/lesson/format.php
mod/quiz/report/attemptsreport_table.php
question/behaviour/behaviourbase.php
question/engine/lib.php
question/engine/questionattempt.php
question/engine/tests/questionutils_test.php
question/format.php
question/format/aiken/format.php
question/format/blackboard/format.php
question/format/blackboard_six/formatpool.php
question/format/blackboard_six/formatqti.php
question/format/examview/format.php
question/format/gift/format.php
question/format/learnwise/format.php
question/format/missingword/format.php
question/format/multianswer/format.php
question/format/multianswer/tests/multianswerformat_test.php
question/format/webct/format.php
question/format/xml/format.php
question/tests/importexport_test.php

index 086983d..38f8856 100644 (file)
 
         $r = html_writer::start_tag('li');
             $r.= html_writer::start_tag('div',array('class'=>'link'));
-                $r.= html_writer::link(clean_param($link,PARAM_URL), s($title), array('onclick'=>'this.target="_blank"'));
+                $r.= html_writer::link($link, s($title), array('onclick'=>'this.target="_blank"'));
             $r.= html_writer::end_tag('div');
 
             if($this->config->display_description && !empty($description)){
index 1986c09..47885c9 100644 (file)
@@ -1158,7 +1158,7 @@ class block_manager {
             $PAGE->set_title($blocktitle . ': ' . $strdeletecheck);
             $PAGE->set_heading($site->fullname);
             echo $OUTPUT->header();
-            $confirmurl = new moodle_url("$deletepage->url?", array('sesskey' => sesskey(), 'bui_deleteid' => $block->instance->id, 'bui_confirm' => 1));
+            $confirmurl = new moodle_url($deletepage->url, array('sesskey' => sesskey(), 'bui_deleteid' => $block->instance->id, 'bui_confirm' => 1));
             $cancelurl = new moodle_url($deletepage->url);
             $yesbutton = new single_button($confirmurl, get_string('yes'));
             $nobutton = new single_button($cancelurl, get_string('no'));
index 9531782..7a81253 100644 (file)
@@ -1101,6 +1101,20 @@ abstract class moodle_database {
      */
     public abstract function get_recordset_sql($sql, array $params=null, $limitfrom=0, $limitnum=0);
 
+    /**
+     * Get all records from a table.
+     *
+     * This method works around potential memory problems and may improve performance,
+     * this method may block access to table until the recordset is closed.
+     *
+     * @param string $table Name of database table.
+     * @return moodle_recordset A moodle_recordset instance {@link function get_recordset}.
+     * @throws dml_exception A DML specific exception is thrown for any errors.
+     */
+    public function export_table_recordset($table) {
+        return $this->get_recordset($table, array());
+    }
+
     /**
      * Get a number of records as an array of objects where all the given conditions met.
      *
index 1a238ba..21404d7 100644 (file)
@@ -905,6 +905,27 @@ class mysqli_native_moodle_database extends moodle_database {
         return $this->create_recordset($result);
     }
 
+    /**
+     * Get all records from a table.
+     *
+     * This method works around potential memory problems and may improve performance,
+     * this method may block access to table until the recordset is closed.
+     *
+     * @param string $table Name of database table.
+     * @return moodle_recordset A moodle_recordset instance {@link function get_recordset}.
+     * @throws dml_exception A DML specific exception is thrown for any errors.
+     */
+    public function export_table_recordset($table) {
+        $sql = $this->fix_table_names("SELECT * FROM {{$table}}");
+
+        $this->query_start($sql, array(), SQL_QUERY_SELECT);
+        // MYSQLI_STORE_RESULT may eat all memory for large tables, unfortunately MYSQLI_USE_RESULT blocks other queries.
+        $result = $this->mysqli->query($sql, MYSQLI_USE_RESULT);
+        $this->query_end($result);
+
+        return $this->create_recordset($result);
+    }
+
     protected function create_recordset($result) {
         return new mysqli_native_moodle_recordset($result);
     }
index 2bb7270..e056b53 100644 (file)
@@ -1300,6 +1300,36 @@ class dml_testcase extends database_driver_testcase {
         // note: fetching nulls, empties, LOBs already tested by test_insert_record() no needed here
     }
 
+    public function test_export_table_recordset() {
+        $DB = $this->tdb;
+        $dbman = $DB->get_manager();
+
+        $table = $this->get_test_table();
+        $tablename = $table->getName();
+
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('course', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0');
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $dbman->create_table($table);
+
+        $ids = array();
+        $ids[] = $DB->insert_record($tablename, array('course' => 3));
+        $ids[] = $DB->insert_record($tablename, array('course' => 5));
+        $ids[] = $DB->insert_record($tablename, array('course' => 4));
+        $ids[] = $DB->insert_record($tablename, array('course' => 3));
+        $ids[] = $DB->insert_record($tablename, array('course' => 2));
+        $ids[] = $DB->insert_record($tablename, array('course' => 1));
+        $ids[] = $DB->insert_record($tablename, array('course' => 0));
+
+        $rs = $DB->export_table_recordset($tablename);
+        $rids = array();
+        foreach ($rs as $record) {
+            $rids[] = $record->id;
+        }
+        $rs->close();
+        $this->assertEquals($ids, $rids, '', 0, 0, true);
+    }
+
     public function test_get_records() {
         $DB = $this->tdb;
         $dbman = $DB->get_manager();
index 7051c82..4d9b7bb 100644 (file)
@@ -144,7 +144,7 @@ abstract class database_exporter {
         $tables = $this->schema->getTables();
         $this->begin_database_export($CFG->version, $CFG->release, date('c'), $description);
         foreach ($tables as $table) {
-            $rs = $this->mdb->get_recordset_sql('SELECT * FROM {'.$table->getName().'}');
+            $rs = $this->mdb->export_table_recordset($table->getName());
             if (!$rs) {
                 throw new ddl_table_missing_exception($table->getName());
             }
@@ -153,6 +153,7 @@ abstract class database_exporter {
                 $this->export_table_data($table, $row);
             }
             $this->finish_table_export($table);
+            $rs->close();
         }
         $this->finish_database_export();
     }
index c8520da..07e0d8b 100644 (file)
@@ -23,43 +23,9 @@ function insertMedia() {
     tinyMCEPopup.close();
 }
 
-function getType(v) {
-    var fo, i, c, el, x, f = document.forms[0];
-
-    fo = ed.getParam("media_types", "flash=swf;flv=flv;shockwave=dcr;qt=mov,qt,mpg,mp3,mp4,mpeg;shockwave=dcr;wmp=avi,wmv,wm,asf,asx,wmx,wvx;rmp=rm,ra,ram").split(';');
-
-    // YouTube
-    if (v.match(/watch\?v=(.+)(.*)/)) {
-        f.src.value = 'http://www.youtube.com/v/' + v.match(/v=(.*)(.*)/)[0].split('=')[1];
-        return 'flash';
-    } else if (v.match(/v\/(.+)(.*)/)) {
-        return 'flash';
-    }
-
-    // Google video
-    if (v.indexOf('http://video.google.com/videoplay?docid=') == 0) {
-        f.src.value = 'http://video.google.com/googleplayer.swf?docId=' + v.substring('http://video.google.com/videoplay?docid='.length) + '&hl=en';
-        return 'flash';
-    }
-
-    for (i=0; i<fo.length; i++) {
-        c = fo[i].split('=');
-
-        el = c[1].split(',');
-        for (x=0; x<el.length; x++)
-            if (v.indexOf('.' + el[x]) != -1)
-                return c[0];
-    }
-
-    return null;
-}
-
-
 function serializeParameters() {
-    var d = document, f = d.forms[0], s = '';
+    var d = document, s = '';
     s += getStr(null, 'src');
-    s += 'width:300,';
-    s += 'height:225,';
 
     // delete the tail comma
     s = s.length > 0 ? s.substring(0, s.length - 1) : s;
@@ -87,10 +53,9 @@ function jsEncode(s) {
 }
 
 function generatePreview(c) {
-    var f = document.forms[0], p = document.getElementById('prev'), h = '', cls, pl, n, type, codebase, wp, hp, nw, nh;
+    var f = document.forms[0], p = document.getElementById('prev');
 
     p.innerHTML = '<!-- x --->';
-    var type = getType(f.src.value);
     var re = new RegExp("(.+)\#(.+)", "i");
     var result = f.src.value.match(re);
     if (result) {
@@ -102,7 +67,7 @@ function generatePreview(c) {
     }
 
     // After constrain
-    pl = serializeParameters();
+    var pl = serializeParameters();
     if (pl == '') {
             p.innerHTML = '';
             return;
@@ -116,22 +81,66 @@ function generatePreview(c) {
     }
 
     pl.src = tinyMCEPopup.editor.documentBaseURI.toAbsolute(pl.src);
-    pl.width = !pl.width ? 100 : pl.width;
-    pl.height = !pl.height ? 100 : pl.height;
-    pl.id = !pl.id ? 'moodlemediaid' : pl.id;
-    pl.name = !pl.name ? 'moodlemedianame' : pl.name;
-    pl.align = !pl.align ? '' : pl.align;
-
-    // Avoid annoying warning about insecure items
-    if (!tinymce.isIE || document.location.protocol != 'https:') {
-        // Include all the draftfile params after the ?
-        var draftparams = pl.src.toString().replace(/^.*\/draftfile.php\//, '');
-        h = '<iframe src="' + tinyMCE.baseURL + '/plugins/moodlemedia/preview.php?path=' +
-                draftparams + '" width="100%" height="100%"></iframe>';
-    }
+    // NOTE: Do not try to prevent https security popups here - users would get them later on real page anyway!
+
+    // We can not include URL directly in parameters because some security filters might block it.
+    p.innerHTML = '<iframe src="' + tinyMCE.baseURL + '/plugins/moodlemedia/preview.php'
+        + '?path=' + encodeURIComponent(encode64(pl.src.toString()))
+        + '&sesskey=' + encodeURIComponent(parent.M.cfg.sesskey)
+        + '" width="100%" height="100%"></iframe>';
+}
 
-    // I don't know why the HTML comment is there, but leaving it just in case
-    p.innerHTML = "<!-- x --->" + h;
+function encode64(input) {
+    /*
+      CryptoMX Tools
+      Copyright (C) 2004 - 2006 Derek Buitenhuis
+
+      This program is free software; you can redistribute it and/or
+      modify it under the terms of the GNU General Public License
+      as published by the Free Software Foundation; either version 2
+      of the License, or (at your option) any later version.
+
+      This program is distributed in the hope that it will be useful,
+      but WITHOUT ANY WARRANTY; without even the implied warranty of
+      MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+      GNU General Public License for more details.
+
+      You should have received a copy of the GNU General Public License
+      along with this program; if not, write to the Free Software
+      Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+   */
+    var keyStr = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
+    var output = "";
+    var chr1, chr2, chr3 = "";
+    var enc1, enc2, enc3, enc4 = "";
+    var i = 0;
+
+    do {
+        chr1 = input.charCodeAt(i++);
+        chr2 = input.charCodeAt(i++);
+        chr3 = input.charCodeAt(i++);
+
+        enc1 = chr1 >> 2;
+        enc2 = ((chr1 & 3) << 4) | (chr2 >> 4);
+        enc3 = ((chr2 & 15) << 2) | (chr3 >> 6);
+        enc4 = chr3 & 63;
+
+        if (isNaN(chr2)) {
+            enc3 = enc4 = 64;
+        } else if (isNaN(chr3)) {
+            enc4 = 64;
+        }
+
+        output = output +
+            keyStr.charAt(enc1) +
+            keyStr.charAt(enc2) +
+            keyStr.charAt(enc3) +
+            keyStr.charAt(enc4);
+        chr1 = chr2 = chr3 = "";
+        enc1 = enc2 = enc3 = enc4 = "";
+    } while (i < input.length);
+
+    return output;
 }
 
 tinyMCEPopup.onInit.add(init);
index f26251a..a49e3c0 100644 (file)
@@ -28,18 +28,22 @@ require_once($CFG->libdir . '/filelib.php');
 require_once($CFG->libdir . '/editorlib.php');
 require_once($CFG->libdir . '/editor/tinymce/lib.php');
 
-// Must be logged in
+// Must be logged in and have a valid session key.
 require_login();
+require_sesskey();
 
-// Require path to draftfile.php file
-$path = required_param('path', PARAM_PATH);
+// URL to the media.
+$path = required_param('path', PARAM_RAW);
+$path = base64_decode($path);
+$url = clean_param($path, PARAM_URL);
+$url = new moodle_url($url);
 
 $editor = new tinymce_texteditor();
 
-// Now output this file which is super-simple
+// Now output this file which is super-simple.
 $PAGE->set_pagelayout('embedded');
 $PAGE->set_url(new moodle_url('/lib/editor/tinymce/tiny_mce/'.$editor->version.'/plugins/moodlemedia/preview.php',
-        array('path' => $path)));
+        array('path' => base64_encode($path))));
 $PAGE->set_context(context_system::instance());
 $PAGE->add_body_class('core_media_preview');
 
@@ -47,14 +51,6 @@ echo $OUTPUT->header();
 
 $mediarenderer = $PAGE->get_renderer('core', 'media');
 
-$path = '/'.trim($path, '/');
-
-if (empty($CFG->slasharguments)) {
-    $url = new moodle_url('/draftfile.php', array('file'=>$path));
-} else {
-    $url = new moodle_url('/draftfile.php');
-    $url->set_slashargument($path);
-}
 if ($mediarenderer->can_embed_url($url)) {
     echo $mediarenderer->embed_url($url);
 }
index df23d91..7c2087e 100644 (file)
@@ -832,7 +832,7 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea
             if ($oldfile->get_contenthash() != $newfile->get_contenthash() || $oldfile->get_filesize() != $newfile->get_filesize()) {
                 $oldfile->replace_content_with($newfile);
                 // push changes to all local files that are referencing this file
-                $fs->update_references_to_storedfile($this);
+                $fs->update_references_to_storedfile($oldfile);
             }
 
             // unchanged file or directory - we keep it as is
index ae8df9c..917705b 100644 (file)
@@ -1821,7 +1821,7 @@ class file_storage {
      * @param stored_file $storedfile
      */
     public function update_references_to_storedfile(stored_file $storedfile) {
-        global $CFG;
+        global $CFG, $DB;
         $params = array();
         $params['contextid'] = $storedfile->get_contextid();
         $params['component'] = $storedfile->get_component();
index 6e7042d..812fe07 100644 (file)
@@ -88,17 +88,17 @@ M.util.CollapsibleRegion = function(Y, id, userpref, strtooltip) {
 
     // Get the caption for the collapsible region
     var caption = this.div.one('#'+id + '_caption');
-    caption.setAttribute('title', strtooltip);
 
     // Create a link
     var a = Y.Node.create('<a href="#"></a>');
-    // Create a local scoped lamba function to move nodes to a new link
-    var movenode = function(node){
-        node.remove();
-        a.append(node);
-    };
-    // Apply the lamba function on each of the captions child nodes
-    caption.get('children').each(movenode, this);
+    a.setAttribute('title', strtooltip);
+
+    // Get all the nodes from caption, remove them and append them to <a>
+    while (caption.hasChildNodes()) {
+        child = caption.get('firstChild');
+        child.remove();
+        a.append(child);
+    }
     caption.append(a);
 
     // Get the height of the div at this point before we shrink it if required
@@ -1445,6 +1445,7 @@ M.util.help_icon = {
                     },
 
                     display_callback : function(content) {
+                        content = '<div role="alert">' + content + '</div>';
                         this.overlay.set('bodyContent', content);
                     },
 
index ce65499..388ded6 100644 (file)
@@ -5959,16 +5959,19 @@ function get_max_upload_sizes($sitebytes = 0, $coursebytes = 0, $modulebytes = 0
         return array();
     }
 
+    $filesize = array();
     $filesize[intval($maxsize)] = display_size($maxsize);
 
     $sizelist = array(10240, 51200, 102400, 512000, 1048576, 2097152,
                       5242880, 10485760, 20971520, 52428800, 104857600);
 
-    // If custombytes is given then add it to the list.
-    if (!is_null($custombytes)) {
-        if (is_number($custombytes)) {
-            $custombytes = array((int)$custombytes);
+    // If custombytes is given and is valid then add it to the list.
+    if (is_number($custombytes) and $custombytes > 0) {
+        $custombytes = (int)$custombytes;
+        if (!in_array($custombytes, $sizelist)) {
+            $sizelist[] = $custombytes;
         }
+    } else if (is_array($custombytes)) {
         $sizelist = array_unique(array_merge($sizelist, $custombytes));
     }
 
index 717ad00..dd03dc8 100644 (file)
@@ -1727,7 +1727,7 @@ class core_renderer extends renderer_base {
         // note: this title is displayed only if JS is disabled, otherwise the link will have the new ajax tooltip
         $title = get_string('helpprefix2', '', trim($helpicon->title, ". \t"));
 
-        $attributes = array('href'=>$url, 'title'=>$title);
+        $attributes = array('href'=>$url, 'title'=>$title, 'aria-haspopup' => 'true');
         $id = html_writer::random_id('helpicon');
         $attributes['id'] = $id;
         $output = html_writer::tag('a', $output, $attributes);
@@ -1792,7 +1792,7 @@ class core_renderer extends renderer_base {
         // note: this title is displayed only if JS is disabled, otherwise the link will have the new ajax tooltip
         $title = get_string('helpprefix2', '', trim($title, ". \t"));
 
-        $attributes = array('href'=>$url, 'title'=>$title);
+        $attributes = array('href'=>$url, 'title'=>$title, 'aria-haspopup' => 'true');
         $id = html_writer::random_id('helpicon');
         $attributes['id'] = $id;
         $output = html_writer::tag('a', $output, $attributes);
index 65e1c35..76e50eb 100644 (file)
@@ -843,6 +843,11 @@ class available_update_checker {
     /**
      * Loads the most recent raw response record we have fetched
      *
+     * After this method is called, $this->recentresponse is set to an array. If the
+     * array is empty, then either no data have been fetched yet or the fetched data
+     * do not have expected format (and thence they are ignored and a debugging
+     * message is displayed).
+     *
      * This implementation uses the config_plugins table as the permanent storage.
      *
      * @param bool $forcereload reload even if it was already loaded
@@ -862,7 +867,8 @@ class available_update_checker {
                 $this->recentfetch = $config->recentfetch;
                 $this->recentresponse = $this->decode_response($config->recentresponse);
             } catch (available_update_checker_exception $e) {
-                // do not set recentresponse if the validation fails
+                debugging('Invalid info about available updates detected and will be ignored: '.$e->getMessage(), DEBUG_ALL);
+                $this->recentresponse = array();
             }
 
         } else {
index fa97208..7d9f0a8 100644 (file)
@@ -94,8 +94,9 @@ class backup_assign_activity_structure_step extends backup_activity_structure_st
         $pluginconfigs->add_child($pluginconfig);
 
 
-        // Define sources
+        // Define sources.
         $assign->set_source_table('assign', array('id' => backup::VAR_ACTIVITYID));
+        $pluginconfig->set_source_table('assign_plugin_config', array('assignment' => backup::VAR_PARENTID));
 
         if ($userinfo) {
             $submission->set_source_table('assign_submission',
@@ -103,15 +104,12 @@ class backup_assign_activity_structure_step extends backup_activity_structure_st
 
             $grade->set_source_table('assign_grades',
                                      array('assignment' => backup::VAR_PARENTID));
-            $pluginconfig->set_source_table('assign_plugin_config',
-                                     array('assignment' => backup::VAR_PARENTID));
 
-            // support 2 types of subplugins
+            // Support 2 types of subplugins.
             $this->add_subplugin_structure('assignsubmission', $submission, true);
             $this->add_subplugin_structure('assignfeedback', $grade, true);
         }
 
-
         // Define id annotations
         $submission->annotate_ids('user', 'userid');
         $grade->annotate_ids('user', 'userid');
index 95bb645..134c14b 100644 (file)
@@ -541,6 +541,37 @@ class qformat_default {
         return NULL;
     }
 
+    /**
+     * Construct a reasonable default question name, based on the start of the question text.
+     * @param string $questiontext the question text.
+     * @param string $default default question name to use if the constructed one comes out blank.
+     * @return string a reasonable question name.
+     */
+    public function create_default_question_name($questiontext, $default) {
+        $name = $this->clean_question_name(shorten_text($questiontext, 80));
+        if ($name) {
+            return $name;
+        } else {
+            return $default;
+        }
+    }
+
+    /**
+     * Ensure that a question name does not contain anything nasty, and will fit in the DB field.
+     * @param string $name the raw question name.
+     * @return string a safe question name.
+     */
+    public function clean_question_name($name) {
+        $name = clean_param($name, PARAM_TEXT); // Matches what the question editing form does.
+        $name = trim($name);
+        $trimlength = 251;
+        while (textlib::strlen($name) > 255 && $trimlength > 0) {
+            $name = shorten_text($name, $trimlength);
+            $trimlength -= 10;
+        }
+        return $name;
+    }
+
     function defaultquestion() {
     // returns an "empty" question
     // Somewhere to specify question parameters that are not handled
index f119da0..4b57900 100644 (file)
@@ -518,7 +518,7 @@ abstract class quiz_attempts_report_table extends table_sql {
             return;
         }
 
-        $url = new moodle_url($this->reporturl, $this->displayoptions);
+        $url = $this->options->get_url();
         $url->param('sesskey', sesskey());
 
         echo '<div id="tablecontainer">';
index cd527ef..1f6beb2 100644 (file)
@@ -477,9 +477,9 @@ abstract class question_behaviour {
      */
     public static function is_manual_grade_in_range($qubaid, $slot) {
         $prefix = 'q' . $qubaid . ':' . $slot . '_';
-        $mark = optional_param($prefix . '-mark', null, PARAM_NUMBER);
-        $maxmark = optional_param($prefix . '-maxmark', null, PARAM_NUMBER);
-        $minfraction = optional_param($prefix . ':minfraction', null, PARAM_NUMBER);
+        $mark = question_utils::optional_param_mark($prefix . '-mark');
+        $maxmark = optional_param($prefix . '-maxmark', null, PARAM_FLOAT);
+        $minfraction = optional_param($prefix . ':minfraction', null, PARAM_FLOAT);
         return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxmark);
     }
 
index d9c7d0f..9fdfc03 100644 (file)
@@ -789,6 +789,33 @@ abstract class question_utils {
         return self::$thousands[$number / 1000 % 10] . self::$hundreds[$number / 100 % 10] .
                 self::$tens[$number / 10 % 10] . self::$units[$number % 10];
     }
+
+    /**
+     * Typically, $mark will have come from optional_param($name, null, PARAM_RAW_TRIMMED).
+     * This method copes with:
+     *  - keeping null or '' input unchanged.
+     *  - nubmers that were typed as either 1.00 or 1,00 form.
+     *
+     * @param string|null $mark raw use input of a mark.
+     * @return float|string|null cleaned mark as a float if possible. Otherwise '' or null.
+     */
+    public static function clean_param_mark($mark) {
+        if ($mark === '' || is_null($mark)) {
+            return $mark;
+        }
+
+        return clean_param(str_replace(',', '.', $mark), PARAM_FLOAT);
+    }
+
+    /**
+     * Get a sumitted variable (from the GET or POST data) that is a mark.
+     * @param string $parname the submitted variable name.
+     * @return float|string|null cleaned mark as a float if possible. Otherwise '' or null.
+     */
+    public static function optional_param_mark($parname) {
+        return self::clean_param_mark(
+                optional_param($parname, null, PARAM_RAW_TRIMMED));
+    }
 }
 
 
index 1be7df2..d89f0f7 100644 (file)
@@ -882,13 +882,8 @@ class question_attempt {
     public function get_submitted_var($name, $type, $postdata = null) {
         switch ($type) {
             case self::PARAM_MARK:
-                // Special case to work around PARAM_NUMBER converting '' to 0.
-                $mark = $this->get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata);
-                if ($mark === '' || is_null($mark)) {
-                    return $mark;
-                } else {
-                    return clean_param(str_replace(',', '.', $mark), PARAM_NUMBER);
-                }
+                // Special case to work around PARAM_FLOAT converting '' to 0.
+                return question_utils::clean_param_mark($this->get_submitted_var($name, PARAM_RAW_TRIMMED, $postdata));
 
             case self::PARAM_FILES:
                 return $this->process_response_files($name, $name, $postdata);
index e51d4db..e5935f4 100644 (file)
@@ -191,4 +191,14 @@ class question_utils_test extends advanced_testcase {
         $this->setExpectedException('moodle_exception');
         question_utils::int_to_roman(1.5);
     }
-}
\ No newline at end of file
+
+    public function test_clean_param_mark() {
+        $this->assertNull(question_utils::clean_param_mark(null));
+        $this->assertSame('', question_utils::clean_param_mark(''));
+        $this->assertSame(0.0, question_utils::clean_param_mark('0'));
+        $this->assertSame(1.5, question_utils::clean_param_mark('1.5'));
+        $this->assertSame(1.5, question_utils::clean_param_mark('1,5'));
+        $this->assertSame(-1.5, question_utils::clean_param_mark('-1.5'));
+        $this->assertSame(-1.5, question_utils::clean_param_mark('-1,5'));
+    }
+}
index dd02cfc..13c1232 100644 (file)
@@ -660,6 +660,37 @@ class qformat_default {
         return $question;
     }
 
+    /**
+     * Construct a reasonable default question name, based on the start of the question text.
+     * @param string $questiontext the question text.
+     * @param string $default default question name to use if the constructed one comes out blank.
+     * @return string a reasonable question name.
+     */
+    public function create_default_question_name($questiontext, $default) {
+        $name = $this->clean_question_name(shorten_text($questiontext, 80));
+        if ($name) {
+            return $name;
+        } else {
+            return $default;
+        }
+    }
+
+    /**
+     * Ensure that a question name does not contain anything nasty, and will fit in the DB field.
+     * @param string $name the raw question name.
+     * @return string a safe question name.
+     */
+    public function clean_question_name($name) {
+        $name = clean_param($name, PARAM_TEXT); // Matches what the question editing form does.
+        $name = trim($name);
+        $trimlength = 251;
+        while (textlib::strlen($name) > 255 && $trimlength > 0) {
+            $name = shorten_text($name, $trimlength);
+            $trimlength -= 10;
+        }
+        return $name;
+    }
+
     /**
      * Add a blank combined feedback to a question object.
      * @param object question
index 4e64c2f..968a211 100644 (file)
@@ -95,7 +95,7 @@ class qformat_aiken extends qformat_default {
                 } else {
                     // Must be the first line of a new question, since no recognised prefix.
                     $question->qtype = MULTICHOICE;
-                    $question->name = shorten_text(s($nowline), 50);
+                    $question->name = $this->create_default_question_name($nowline, get_string('questionname', 'question'));
                     $question->questiontext = htmlspecialchars(trim($nowline), ENT_NOQUOTES);
                     $question->questiontextformat = FORMAT_HTML;
                     $question->generalfeedback = '';
index 1e7fa0d..9d30323 100644 (file)
@@ -123,13 +123,9 @@ class qformat_blackboard extends qformat_based_on_xml {
             $question->questiontext = $text;
         }
         // Put name in question object. We must ensure it is not empty and it is less than 250 chars.
-        $question->name = shorten_text(strip_tags($question->questiontext), 200);
-        $question->name = substr($question->name, 0, 250);
-        if (!$question->name) {
-            $id = $this->getpath($questiondata,
-                    array('@', 'id'), '',  true);
-            $question->name = get_string('defaultname', 'qformat_blackboard' , $id);
-        }
+        $id = $this->getpath($questiondata, array('@', 'id'), '',  true);
+        $question->name = $this->create_default_question_name($question->questiontext,
+                get_string('defaultname', 'qformat_blackboard', $id));
 
         $question->generalfeedback = '';
         $question->generalfeedbackformat = FORMAT_HTML;
index 80867f3..a6c942c 100644 (file)
@@ -92,13 +92,9 @@ class qformat_blackboard_six_pool extends qformat_blackboard_six_base {
         $question->questiontextformat = FORMAT_HTML; // Needed because add_blank_combined_feedback uses it.
 
         // Put name in question object. We must ensure it is not empty and it is less than 250 chars.
-        $question->name = shorten_text(strip_tags($question->questiontext['text']), 200);
-        $question->name = substr($question->name, 0, 250);
-        if (!$question->name) {
-            $id = $this->getpath($questiondata,
-                    array('@', 'id'), '',  true);
-            $question->name = get_string('defaultname', 'qformat_blackboard_six' , $id);
-        }
+        $id = $this->getpath($questiondata, array('@', 'id'), '',  true);
+        $question->name = $this->create_default_question_name($question->questiontext['text'],
+                get_string('defaultname', 'qformat_blackboard_six' , $id));
 
         $question->generalfeedback = '';
         $question->generalfeedbackformat = FORMAT_HTML;
index 223d9f6..37545fd 100644 (file)
@@ -510,11 +510,8 @@ class qformat_blackboard_six_qti extends qformat_blackboard_six_base {
         $question->questiontext = $this->cleaned_text_field($text);
         $question->questiontextformat = FORMAT_HTML; // Needed because add_blank_combined_feedback uses it.
 
-        $question->name = shorten_text(strip_tags($question->questiontext['text']), 200);
-        $question->name = substr($question->name, 0, 250);
-        if (!$question->name) {
-            $question->name = get_string('defaultname', 'qformat_blackboard_six' , $quest->id);
-        }
+        $question->name = $this->create_default_question_name($question->questiontext['text'],
+                get_string('defaultname', 'qformat_blackboard_six' , $quest->id));
         $question->generalfeedback = '';
         $question->generalfeedbackformat = FORMAT_HTML;
         $question->generalfeedbackfiles = array();
index 9cb9bb5..2f3eccd 100644 (file)
@@ -131,7 +131,7 @@ class qformat_examview extends qformat_based_on_xml {
             $question->questiontext = $htmltext;
             $question->questiontextformat = FORMAT_HTML;
             $question->questiontextfiles = array();
-            $question->name = shorten_text( $question->questiontext, 250 );
+            $question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
             $question->qtype = MATCH;
             $question = $this->add_blank_combined_feedback($question);
             $question->subquestions = array();
@@ -200,7 +200,7 @@ class qformat_examview extends qformat_based_on_xml {
         $question->questiontext = $this->cleaninput($htmltext);
         $question->questiontextformat = FORMAT_HTML;
         $question->questiontextfiles = array();
-        $question->name = shorten_text( $question->questiontext, 250 );
+        $question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
 
         switch ($question->qtype) {
             case MULTICHOICE:
index 1cb0e25..ecca6e2 100644 (file)
@@ -206,7 +206,7 @@ class qformat_gift extends qformat_default {
                 // name will be assigned after processing question text below
             } else {
                 $questionname = substr($text, 0, $namefinish);
-                $question->name = trim($this->escapedchar_post($questionname));
+                $question->name = $this->clean_question_name($this->escapedchar_post($questionname));
                 $text = trim(substr($text, $namefinish+2)); // Remove name from text
             }
         } else {
@@ -251,13 +251,9 @@ class qformat_gift extends qformat_default {
 
         // set question name if not already set
         if ($question->name === false) {
-            $question->name = $question->questiontext;
+            $question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
         }
 
-        // ensure name is not longer than 250 characters
-        $question->name = shorten_text($question->name, 200);
-        $question->name = strip_tags(substr($question->name, 0, 250));
-
         // determine QUESTION TYPE
         $question->qtype = NULL;
 
index fa80475..ae80f13 100644 (file)
@@ -126,10 +126,7 @@ class qformat_learnwise extends qformat_default {
 
         $question = $this->defaultquestion();
         $question->qtype = MULTICHOICE;
-        $question->name = substr($questiontext, 0, 30);
-        if (strlen($questiontext) > 30) {
-            $question->name .= '...';
-        }
+        $question->name = $this->create_default_question_name($questiontext, get_string('questionname', 'question'));
 
         $question->questiontext = $questiontext;
         $question->single = ($type == 'multichoice') ? 1 : 0;
index 15e1d71..d4347bc 100644 (file)
@@ -89,7 +89,7 @@ class qformat_missingword extends qformat_default {
 
         /// Save the new question text
         $question->questiontext = substr_replace($text, "_____", $answerstart, $answerlength+1);
-        $question->name = $question->questiontext;
+        $question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
 
 
         /// Parse the answers
index d7a7ac7..b7c412e 100644 (file)
@@ -62,18 +62,7 @@ class qformat_multianswer extends qformat_default {
         $question->penalty = 0.3333333;
 
         if (!empty($question)) {
-            $name = html_to_text(implode(' ', $lines));
-            $name = preg_replace('/{[^}]*}/', '', $name);
-            $name = trim($name);
-
-            if ($name) {
-                $question->name = shorten_text($name, 45);
-            } else {
-                // We need some name, so use the current time, since that will be
-                // reasonably unique.
-                $question->name = userdate(time());
-            }
-
+            $question->name = $this->create_default_question_name($question->questiontext, get_string('questionname', 'question'));
             $questions[] = $question;
         }
 
index 29c3224..8e1d301 100644 (file)
@@ -47,7 +47,9 @@ class qformat_multianswer_test extends question_testcase {
         $qs = $importer->readquestions($lines);
 
         $expectedq = (object) array(
-            'name' => 'Match the following cities with the ...',
+            'name' => 'Match the following cities with the correct state:
+* San Francisco: {#1}
+* ...',
             'questiontext' => 'Match the following cities with the correct state:
 * San Francisco: {#1}
 * Tucson: {#2}
index c599ca8..28841e8 100644 (file)
@@ -259,11 +259,8 @@ class qformat_webct extends qformat_default {
 
                     // Setup default value of missing fields
                     if (!isset($question->name)) {
-                        $question->name = $question->questiontext;
-                    }
-                    if (strlen($question->name) > 255) {
-                        $question->name = substr($question->name,0,250)."...";
-                        $warnings[] = get_string("questionnametoolong", "qformat_webct", $nQuestionStartLine);
+                        $question->name = $this->create_default_question_name(
+                                $question->questiontext, get_string('questionname', 'question'));
                     }
                     if (!isset($question->defaultmark)) {
                         $question->defaultmark = 1;
@@ -466,11 +463,7 @@ class qformat_webct extends qformat_default {
 
             if (preg_match("~^:TITLE:(.*)~i",$line,$webct_options)) {
                 $name = trim($webct_options[1]);
-                if (strlen($name) > 255) {
-                    $name = substr($name,0,250)."...";
-                    $warnings[] = get_string("questionnametoolong", "qformat_webct", $nLineCounter);
-                }
-                $question->name = $name;
+                $question->name = $this->clean_question_name($name);
                 continue;
             }
 
index 740279f..1a0e992 100644 (file)
@@ -166,9 +166,9 @@ class qformat_xml extends qformat_default {
         $qo = $this->defaultquestion();
 
         // Question name
-        $qo->name = $this->getpath($question,
+        $qo->name = $this->clean_question_name($this->getpath($question,
                 array('#', 'name', 0, '#', 'text', 0, '#'), '', true,
-                get_string('xmlimportnoname', 'qformat_xml'));
+                get_string('xmlimportnoname', 'qformat_xml')));
         $qo->questiontext = $this->getpath($question,
                 array('#', 'questiontext', 0, '#', 'text', 0, '#'), '', true);
         $qo->questiontextformat = $this->trans_format($this->getpath(
@@ -437,7 +437,7 @@ class qformat_xml extends qformat_default {
         $qo->course = $this->course;
         $qo->generalfeedback = '';
 
-        $qo->name = $this->import_text($question['#']['name'][0]['#']['text']);
+        $qo->name = $this->clean_question_name($this->import_text($question['#']['name'][0]['#']['text']));
         $qo->questiontextformat = $questiontext['format'];
         $qo->questiontext = $qo->questiontext['text'];
         $qo->questiontextfiles = array();
index 0ffe435..96d0704 100644 (file)
@@ -87,4 +87,71 @@ class qformat_default_test extends advanced_testcase {
         $path = '<evil>Nasty <virus //> thing<//evil>';
         $this->assertEquals(array('Nasty  thing'), $format->split_category_path($path));
     }
+
+    public function test_clean_question_name() {
+        $format = new testable_qformat();
+
+        $name = 'Nice simple name';
+        $this->assertEquals($name, $format->clean_question_name($name));
+
+        $name = 'Question in <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>';
+        $this->assertEquals($name, $format->clean_question_name($name));
+
+        $name = 'Evil <script type="text/javascrip">alert("You have been hacked!");</script>';
+        $this->assertEquals('Evil alert("You have been hacked!");', $format->clean_question_name($name));
+
+        $name = 'This is a very long question name. It goes on and on and on. ' .
+                'I wonder if it will ever stop. The quetsion name field in the database is only ' .
+                'two hundred and fifty five characters wide, so if the import file contains a ' .
+                'name longer than that, the code had better truncate it!';
+        $this->assertEquals(shorten_text($name, 251), $format->clean_question_name($name));
+
+        // The worst case scenario is a whole lot of single charaters in separate multilang tags.
+        $name = '<span lang="en" class="multilang">A</span>' .
+                '<span lang="fr" class="multilang">B</span>' .
+                '<span lang="fr_ca" class="multilang">C</span>' .
+                '<span lang="en_us" class="multilang">D</span>' .
+                '<span lang="de" class="multilang">E</span>' .
+                '<span lang="cz" class="multilang">F</span>' .
+                '<span lang="it" class="multilang">G</span>' .
+                '<span lang="es" class="multilang">H</span>' .
+                '<span lang="pt" class="multilang">I</span>' .
+                '<span lang="ch" class="multilang">J</span>';
+        $this->assertEquals(shorten_text($name, 1), $format->clean_question_name($name));
+    }
+
+    public function test_create_default_question_name() {
+        $format = new testable_qformat();
+
+        $text = 'Nice simple name';
+        $this->assertEquals($text, $format->create_default_question_name($text, 'Default'));
+
+        $this->assertEquals('Default', $format->create_default_question_name('', 'Default'));
+
+        $text = 'Question in <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>';
+        $this->assertEquals($text, $format->create_default_question_name($text, 'Default'));
+
+        $text = 'Evil <script type="text/javascrip">alert("You have been hacked!");</script>';
+        $this->assertEquals('Evil alert("You have been hacked!");',
+                $format->create_default_question_name($text, 'Default'));
+
+        $text = 'This is a very long question text. It goes on and on and on. ' .
+                'I wonder if it will ever stop. The question name field in the database is only ' .
+                'two hundred and fifty five characters wide, so if the import file contains a ' .
+                'name longer than that, the code had better truncate it!';
+        $this->assertEquals(shorten_text($text, 80), $format->create_default_question_name($text, 'Default'));
+
+        // The worst case scenario is a whole lot of single charaters in separate multilang tags.
+        $text = '<span lang="en" class="multilang">A</span>' .
+                '<span lang="fr" class="multilang">B</span>' .
+                '<span lang="fr_ca" class="multilang">C</span>' .
+                '<span lang="en_us" class="multilang">D</span>' .
+                '<span lang="de" class="multilang">E</span>' .
+                '<span lang="cz" class="multilang">F</span>' .
+                '<span lang="it" class="multilang">G</span>' .
+                '<span lang="es" class="multilang">H</span>' .
+                '<span lang="pt" class="multilang">I</span>' .
+                '<span lang="ch" class="multilang">J</span>';
+        $this->assertEquals(shorten_text($text, 1), $format->create_default_question_name($text, 'Default'));
+    }
 }