diff --git a/completion/classes/api.php b/completion/classes/api.php index b569e3791d1..f55aa90ca06 100644 --- a/completion/classes/api.php +++ b/completion/classes/api.php @@ -87,7 +87,8 @@ class api { if ($completionexpectedtime !== null) { // Calendar event exists so update it. $event->name = get_string('completionexpectedfor', 'completion', $lang); - $event->description = format_module_intro($modulename, $instance, $cmid); + $event->description = format_module_intro($modulename, $instance, $cmid, false); + $event->format = FORMAT_HTML; $event->timestart = $completionexpectedtime; $event->timesort = $completionexpectedtime; $event->visible = instance_is_visible($modulename, $instance); @@ -104,7 +105,8 @@ class api { // Event doesn't exist so create one. if ($completionexpectedtime !== null) { $event->name = get_string('completionexpectedfor', 'completion', $lang); - $event->description = format_module_intro($modulename, $instance, $cmid); + $event->description = format_module_intro($modulename, $instance, $cmid, false); + $event->format = FORMAT_HTML; $event->courseid = $instance->course; $event->groupid = 0; $event->userid = 0; diff --git a/lib/pagelib.php b/lib/pagelib.php index 7e173071f40..99366fc3872 100644 --- a/lib/pagelib.php +++ b/lib/pagelib.php @@ -1572,6 +1572,15 @@ class moodle_page { $this->_wherethemewasinitialised = debug_backtrace(); } + /** + * For diagnostic/debugging purposes, find where the theme setup was triggered. + * + * @return null|array null if theme not yet setup. Stacktrace if it was. + */ + public function get_where_theme_was_initialised() { + return $this->_wherethemewasinitialised; + } + /** * Reset the theme and output for a new context. This only makes sense from * external::validate_context(). Do not cheat. diff --git a/lib/testing/generator/block_generator.php b/lib/testing/generator/block_generator.php index ce5da1fc99a..3c4d9a480a5 100644 --- a/lib/testing/generator/block_generator.php +++ b/lib/testing/generator/block_generator.php @@ -114,10 +114,14 @@ abstract class testing_block_generator extends component_generator_base { * @return stdClass the block_instance record that has just been created. */ public function create_instance($record = null, $options = array()) { - global $DB; + global $DB, $PAGE; $this->instancecount++; + // Creating a block is a back end operation, which should not cause any output to happen. + // This will allow us to check that the theme was not initialised while creating the block instance. + $outputstartedbefore = $PAGE->get_where_theme_was_initialised(); + $record = (object)(array)$record; $this->preprocess_record($record, $options); $record = $this->prepare_record($record); @@ -133,6 +137,19 @@ abstract class testing_block_generator extends component_generator_base { context_block::instance($id); $instance = $DB->get_record('block_instances', array('id' => $id), '*', MUST_EXIST); + + // If the theme was initialised while creating the block instance, something somewhere called an output + // function. Rather than leaving this as a hard-to-debug situation, let's make it fail with a clear error. + $outputstartedafter = $PAGE->get_where_theme_was_initialised(); + + if ($outputstartedbefore === null && $outputstartedafter !== null) { + throw new coding_exception('Creating a block_' . $this->get_blockname() . ' initialised the theme and output!', + 'This should not happen. Creating a block should be a pure back-end operation. Unnecessarily initialising ' . + 'the output mechanism at the wrong time can cause subtle bugs and is a significant performance hit. There is ' . + 'likely a call to an output function that caused it:' . PHP_EOL . PHP_EOL . + format_backtrace($outputstartedafter, true)); + } + return $instance; } diff --git a/lib/testing/generator/module_generator.php b/lib/testing/generator/module_generator.php index 5400648bc56..4a2396e552a 100644 --- a/lib/testing/generator/module_generator.php +++ b/lib/testing/generator/module_generator.php @@ -222,11 +222,15 @@ abstract class testing_module_generator extends component_generator_base { * cmid (corresponding id in course_modules table) */ public function create_instance($record = null, array $options = null) { - global $CFG, $DB; + global $CFG, $DB, $PAGE; require_once($CFG->dirroot.'/course/modlib.php'); $this->instancecount++; + // Creating an activity is a back end operation, which should not cause any output to happen. + // This will allow us to check that the theme was not initialised while creating the module instance. + $outputstartedbefore = $PAGE->get_where_theme_was_initialised(); + // Merge options into record and add default values. $record = $this->prepare_moduleinfo_record($record, $options); @@ -269,6 +273,19 @@ abstract class testing_module_generator extends component_generator_base { // Prepare object to return with additional field cmid. $instance = $DB->get_record($this->get_modulename(), array('id' => $moduleinfo->instance), '*', MUST_EXIST); $instance->cmid = $moduleinfo->coursemodule; + + // If the theme was initialised while creating the module instance, something somewhere called an output + // function. Rather than leaving this as a hard-to-debug situation, let's make it fail with a clear error. + $outputstartedafter = $PAGE->get_where_theme_was_initialised(); + + if ($outputstartedbefore === null && $outputstartedafter !== null) { + throw new coding_exception('Creating a mod_' . $this->get_modulename() . ' activity initialised the theme and output!', + 'This should not happen. Creating an activity should be a pure back-end operation. Unnecessarily initialising ' . + 'the output mechanism at the wrong time can cause subtle bugs and is a significant performance hit. There is ' . + 'likely a call to an output function that caused it:' . PHP_EOL . PHP_EOL . + format_backtrace($outputstartedafter, true)); + } + return $instance; } diff --git a/lib/testing/generator/repository_generator.php b/lib/testing/generator/repository_generator.php index 3d5f560eaaf..21a40663a23 100644 --- a/lib/testing/generator/repository_generator.php +++ b/lib/testing/generator/repository_generator.php @@ -111,12 +111,16 @@ class testing_repository_generator extends component_generator_base { * @return stdClass repository instance record */ public function create_instance($record = null, array $options = null) { - global $CFG, $DB; + global $CFG, $DB, $PAGE; require_once($CFG->dirroot . '/repository/lib.php'); $this->instancecount++; $record = (array) $record; + // Creating a repository is a back end operation, which should not cause any output to happen. + // This will allow us to check that the theme was not initialised while creating the repository instance. + $outputstartedbefore = $PAGE->get_where_theme_was_initialised(); + $typeid = $DB->get_field('repository', 'id', array('type' => $this->get_typename()), MUST_EXIST); $instanceoptions = repository::static_function($this->get_typename(), 'get_instance_option_names'); @@ -146,6 +150,18 @@ class testing_repository_generator extends component_generator_base { $id = repository::static_function($this->get_typename(), 'create', $this->get_typename(), 0, $context, $record); } + // If the theme was initialised while creating the repository instance, something somewhere called an output + // function. Rather than leaving this as a hard-to-debug situation, let's make it fail with a clear error. + $outputstartedafter = $PAGE->get_where_theme_was_initialised(); + + if ($outputstartedbefore === null && $outputstartedafter !== null) { + throw new coding_exception('Creating a repository_' . $this->get_typename() . ' initialised the theme and output!', + 'This should not happen. Creating a repository should be a pure back-end operation. Unnecessarily initialising ' . + 'the output mechanism at the wrong time can cause subtle bugs and is a significant performance hit. There is ' . + 'likely a call to an output function that caused it:' . PHP_EOL . PHP_EOL . + format_backtrace($outputstartedafter, true)); + } + return $DB->get_record('repository_instances', array('id' => $id), '*', MUST_EXIST); } diff --git a/mod/assign/lib.php b/mod/assign/lib.php index 5f65f2e846b..d9ded666cd7 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -297,7 +297,8 @@ function assign_update_events($assign, $override = null) { $event = new stdClass(); $event->type = CALENDAR_EVENT_TYPE_ACTION; - $event->description = format_module_intro('assign', $assigninstance, $cmid); + $event->description = format_module_intro('assign', $assigninstance, $cmid, false); + $event->format = FORMAT_HTML; // Events module won't show user events when the courseid is nonzero. $event->courseid = ($userid) ? 0 : $assigninstance->course; $event->groupid = $groupid; diff --git a/mod/chat/lib.php b/mod/chat/lib.php index 503023c1ef9..478fe8be7b5 100644 --- a/mod/chat/lib.php +++ b/mod/chat/lib.php @@ -123,7 +123,8 @@ function chat_add_instance($chat) { $event = new stdClass(); $event->type = CALENDAR_EVENT_TYPE_ACTION; $event->name = $chat->name; - $event->description = format_module_intro('chat', $chat, $chat->coursemodule); + $event->description = format_module_intro('chat', $chat, $chat->coursemodule, false); + $event->format = FORMAT_HTML; $event->courseid = $chat->course; $event->groupid = 0; $event->userid = 0; @@ -169,7 +170,8 @@ function chat_update_instance($chat) { if ($chat->schedule > 0) { $event->type = CALENDAR_EVENT_TYPE_ACTION; $event->name = $chat->name; - $event->description = format_module_intro('chat', $chat, $chat->coursemodule); + $event->description = format_module_intro('chat', $chat, $chat->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $chat->chattime; $event->timesort = $chat->chattime; @@ -186,7 +188,8 @@ function chat_update_instance($chat) { $event = new stdClass(); $event->type = CALENDAR_EVENT_TYPE_ACTION; $event->name = $chat->name; - $event->description = format_module_intro('chat', $chat, $chat->coursemodule); + $event->description = format_module_intro('chat', $chat, $chat->coursemodule, false); + $event->format = FORMAT_HTML; $event->courseid = $chat->course; $event->groupid = 0; $event->userid = 0; @@ -460,7 +463,8 @@ function chat_prepare_update_events($chat, $cm = null) { $event = new stdClass(); $event->name = $chat->name; $event->type = CALENDAR_EVENT_TYPE_ACTION; - $event->description = format_module_intro('chat', $chat, $cm->id); + $event->description = format_module_intro('chat', $chat, $cm->id, false); + $event->format = FORMAT_HTML; $event->timestart = $chat->chattime; $event->timesort = $chat->chattime; if ($event->id = $DB->get_field('event', 'id', array('modulename' => 'chat', 'instance' => $chat->id, diff --git a/mod/choice/locallib.php b/mod/choice/locallib.php index b5cb29b8213..a00641d034a 100644 --- a/mod/choice/locallib.php +++ b/mod/choice/locallib.php @@ -52,7 +52,8 @@ function choice_set_events($choice) { if ((!empty($choice->timeopen)) && ($choice->timeopen > 0)) { // Calendar event exists so update it. $event->name = get_string('calendarstart', 'choice', $choice->name); - $event->description = format_module_intro('choice', $choice, $choice->coursemodule); + $event->description = format_module_intro('choice', $choice, $choice->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $choice->timeopen; $event->timesort = $choice->timeopen; $event->visible = instance_is_visible('choice', $choice); @@ -68,7 +69,8 @@ function choice_set_events($choice) { // Event doesn't exist so create one. if ((!empty($choice->timeopen)) && ($choice->timeopen > 0)) { $event->name = get_string('calendarstart', 'choice', $choice->name); - $event->description = format_module_intro('choice', $choice, $choice->coursemodule); + $event->description = format_module_intro('choice', $choice, $choice->coursemodule, false); + $event->format = FORMAT_HTML; $event->courseid = $choice->course; $event->groupid = 0; $event->userid = 0; @@ -91,7 +93,8 @@ function choice_set_events($choice) { if ((!empty($choice->timeclose)) && ($choice->timeclose > 0)) { // Calendar event exists so update it. $event->name = get_string('calendarend', 'choice', $choice->name); - $event->description = format_module_intro('choice', $choice, $choice->coursemodule); + $event->description = format_module_intro('choice', $choice, $choice->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $choice->timeclose; $event->timesort = $choice->timeclose; $event->visible = instance_is_visible('choice', $choice); @@ -107,7 +110,8 @@ function choice_set_events($choice) { // Event doesn't exist so create one. if ((!empty($choice->timeclose)) && ($choice->timeclose > 0)) { $event->name = get_string('calendarend', 'choice', $choice->name); - $event->description = format_module_intro('choice', $choice, $choice->coursemodule); + $event->description = format_module_intro('choice', $choice, $choice->coursemodule, false); + $event->format = FORMAT_HTML; $event->courseid = $choice->course; $event->groupid = 0; $event->userid = 0; diff --git a/mod/data/locallib.php b/mod/data/locallib.php index b8bad674fcd..c91540845d6 100644 --- a/mod/data/locallib.php +++ b/mod/data/locallib.php @@ -607,7 +607,8 @@ function data_set_events($data) { if ($data->timeavailablefrom > 0) { // Calendar event exists so update it. $event->name = get_string('calendarstart', 'data', $data->name); - $event->description = format_module_intro('data', $data, $data->coursemodule); + $event->description = format_module_intro('data', $data, $data->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $data->timeavailablefrom; $event->timesort = $data->timeavailablefrom; $event->visible = instance_is_visible('data', $data); @@ -623,7 +624,8 @@ function data_set_events($data) { // Event doesn't exist so create one. if (isset($data->timeavailablefrom) && $data->timeavailablefrom > 0) { $event->name = get_string('calendarstart', 'data', $data->name); - $event->description = format_module_intro('data', $data, $data->coursemodule); + $event->description = format_module_intro('data', $data, $data->coursemodule, false); + $event->format = FORMAT_HTML; $event->courseid = $data->course; $event->groupid = 0; $event->userid = 0; @@ -646,7 +648,8 @@ function data_set_events($data) { if ($data->timeavailableto > 0) { // Calendar event exists so update it. $event->name = get_string('calendarend', 'data', $data->name); - $event->description = format_module_intro('data', $data, $data->coursemodule); + $event->description = format_module_intro('data', $data, $data->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $data->timeavailableto; $event->timesort = $data->timeavailableto; $event->visible = instance_is_visible('data', $data); @@ -662,7 +665,8 @@ function data_set_events($data) { // Event doesn't exist so create one. if (isset($data->timeavailableto) && $data->timeavailableto > 0) { $event->name = get_string('calendarend', 'data', $data->name); - $event->description = format_module_intro('data', $data, $data->coursemodule); + $event->description = format_module_intro('data', $data, $data->coursemodule, false); + $event->format = FORMAT_HTML; $event->courseid = $data->course; $event->groupid = 0; $event->userid = 0; diff --git a/mod/feedback/lib.php b/mod/feedback/lib.php index a3186411ca0..458cb7315bd 100644 --- a/mod/feedback/lib.php +++ b/mod/feedback/lib.php @@ -809,7 +809,8 @@ function feedback_set_events($feedback) { $event->eventtype = FEEDBACK_EVENT_TYPE_OPEN; $event->type = empty($feedback->timeclose) ? CALENDAR_EVENT_TYPE_ACTION : CALENDAR_EVENT_TYPE_STANDARD; $event->name = get_string('calendarstart', 'feedback', $feedback->name); - $event->description = format_module_intro('feedback', $feedback, $feedback->coursemodule); + $event->description = format_module_intro('feedback', $feedback, $feedback->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $feedback->timeopen; $event->timesort = $feedback->timeopen; $event->visible = instance_is_visible('feedback', $feedback); @@ -844,7 +845,8 @@ function feedback_set_events($feedback) { $event->type = CALENDAR_EVENT_TYPE_ACTION; $event->eventtype = FEEDBACK_EVENT_TYPE_CLOSE; $event->name = get_string('calendarend', 'feedback', $feedback->name); - $event->description = format_module_intro('feedback', $feedback, $feedback->coursemodule); + $event->description = format_module_intro('feedback', $feedback, $feedback->coursemodule, false); + $event->format = FORMAT_HTML; $event->timestart = $feedback->timeclose; $event->timesort = $feedback->timeclose; $event->visible = instance_is_visible('feedback', $feedback); diff --git a/mod/forum/locallib.php b/mod/forum/locallib.php index ab333a993ae..d13042dc31a 100644 --- a/mod/forum/locallib.php +++ b/mod/forum/locallib.php @@ -723,7 +723,8 @@ function forum_update_calendar($forum, $cmid) { if (!empty($forum->duedate)) { $event->name = get_string('calendardue', 'forum', $forum->name); - $event->description = format_module_intro('forum', $forum, $cmid); + $event->description = format_module_intro('forum', $forum, $cmid, false); + $event->format = FORMAT_HTML; $event->courseid = $forum->course; $event->modulename = 'forum'; $event->instance = $forum->id; diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php index 1758dc3ff92..45444fcc958 100644 --- a/mod/lesson/lib.php +++ b/mod/lesson/lib.php @@ -162,7 +162,8 @@ function lesson_update_events($lesson, $override = null) { $event = new stdClass(); $event->type = !$deadline ? CALENDAR_EVENT_TYPE_ACTION : CALENDAR_EVENT_TYPE_STANDARD; - $event->description = format_module_intro('lesson', $lesson, $cmid); + $event->description = format_module_intro('lesson', $lesson, $cmid, false); + $event->format = FORMAT_HTML; // Events module won't show user events when the courseid is nonzero. $event->courseid = ($userid) ? 0 : $lesson->course; $event->groupid = $groupid; diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 20fe537a6be..7330c3ef9fa 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -1260,7 +1260,8 @@ function quiz_update_events($quiz, $override = null) { $event = new stdClass(); $event->type = !$timeclose ? CALENDAR_EVENT_TYPE_ACTION : CALENDAR_EVENT_TYPE_STANDARD; - $event->description = format_module_intro('quiz', $quiz, $cmid); + $event->description = format_module_intro('quiz', $quiz, $cmid, false); + $event->format = FORMAT_HTML; // Events module won't show user events when the courseid is nonzero. $event->courseid = ($userid) ? 0 : $quiz->course; $event->groupid = $groupid; diff --git a/mod/scorm/locallib.php b/mod/scorm/locallib.php index f2dd5367014..6647a2baa96 100644 --- a/mod/scorm/locallib.php +++ b/mod/scorm/locallib.php @@ -2424,7 +2424,8 @@ function scorm_update_calendar(stdClass $scorm, $cmid) { if ((!empty($scorm->timeopen)) && ($scorm->timeopen > 0)) { // Calendar event exists so update it. $event->name = get_string('calendarstart', 'scorm', $scorm->name); - $event->description = format_module_intro('scorm', $scorm, $cmid); + $event->description = format_module_intro('scorm', $scorm, $cmid, false); + $event->format = FORMAT_HTML; $event->timestart = $scorm->timeopen; $event->timesort = $scorm->timeopen; $event->visible = instance_is_visible('scorm', $scorm); @@ -2441,7 +2442,8 @@ function scorm_update_calendar(stdClass $scorm, $cmid) { // Event doesn't exist so create one. if ((!empty($scorm->timeopen)) && ($scorm->timeopen > 0)) { $event->name = get_string('calendarstart', 'scorm', $scorm->name); - $event->description = format_module_intro('scorm', $scorm, $cmid); + $event->description = format_module_intro('scorm', $scorm, $cmid, false); + $event->format = FORMAT_HTML; $event->courseid = $scorm->course; $event->groupid = 0; $event->userid = 0; @@ -2465,7 +2467,8 @@ function scorm_update_calendar(stdClass $scorm, $cmid) { if ((!empty($scorm->timeclose)) && ($scorm->timeclose > 0)) { // Calendar event exists so update it. $event->name = get_string('calendarend', 'scorm', $scorm->name); - $event->description = format_module_intro('scorm', $scorm, $cmid); + $event->description = format_module_intro('scorm', $scorm, $cmid, false); + $event->format = FORMAT_HTML; $event->timestart = $scorm->timeclose; $event->timesort = $scorm->timeclose; $event->visible = instance_is_visible('scorm', $scorm); @@ -2482,7 +2485,8 @@ function scorm_update_calendar(stdClass $scorm, $cmid) { // Event doesn't exist so create one. if ((!empty($scorm->timeclose)) && ($scorm->timeclose > 0)) { $event->name = get_string('calendarend', 'scorm', $scorm->name); - $event->description = format_module_intro('scorm', $scorm, $cmid); + $event->description = format_module_intro('scorm', $scorm, $cmid, false); + $event->format = FORMAT_HTML; $event->courseid = $scorm->course; $event->groupid = 0; $event->userid = 0; diff --git a/mod/upgrade.txt b/mod/upgrade.txt index 3c25f25c70e..998bce0963d 100644 --- a/mod/upgrade.txt +++ b/mod/upgrade.txt @@ -5,6 +5,27 @@ information provided here is intended especially for developers. * The callback get_shortcuts() is now deprecated. Please use get_course_content_items and get_all_content_items instead. See source code examples in get_course_content_items() and get_all_content_items() in mod/lti/lib.php for details. +* When creating the calendar events and setting the event description to match the module intro description, the filters + must not be applied on the passed description text. Doing so leads to loosing some expected text filters features and + causes unnecessarily early theme and output initialisation in unit tests. If your activity creates calendar events, + you probably have code like: + ``` + $event->description = format_module_intro('quiz', $quiz, $cmid); + ``` + You need to change it to: + ``` + $event->description = format_module_intro('quiz', $quiz, $cmid, false); + $event->format = FORMAT_HTML; + ``` + Even this is still technically wrong. Content should normally only be formatted just before it is output. Ideally, we + should pass the raw description text, format and have a way to copy the embedded files; or provide another way for the + calendar to call the right format_text() later. The calendar API does not allow us to do these things easily at the + moment. Therefore, this compromise approach is used. The false parameter added ensures that text filters are not run + at this time which is important. And the format must be set to HTML, because otherwise it would use the current user's + preferred editor default format. +* Related to the above and to help with detecting the problematic places in contributed 3rd party modules, the + testing_module_generator::create_instance() now throws coding_exception if creating a module instance initialised the + theme and output as a side effect. === 3.8 === diff --git a/mod/workshop/lib.php b/mod/workshop/lib.php index c38199af69f..951b63174a0 100644 --- a/mod/workshop/lib.php +++ b/mod/workshop/lib.php @@ -1668,6 +1668,7 @@ function workshop_calendar_update(stdClass $workshop, $cmid) { // the common properties for all events $base = new stdClass(); $base->description = format_module_intro('workshop', $workshop, $cmid, false); + $base->format = FORMAT_HTML; $base->courseid = $workshop->course; $base->groupid = 0; $base->userid = 0;