diff --git a/locallib.php b/locallib.php index a1c9899..7d2889c 100644 --- a/locallib.php +++ b/locallib.php @@ -52,15 +52,19 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { // We need a way to discover if we are loading remote docs inside an iframe. $moodleurl->param('embed', 1); + // Reveal eXeLearning's teacher-only content via the package's own URL parameter + // (eXeLearning core hides teacher content by default and opts in to reveal with + // ?exe-teacher=1; see upstream exelearning#1772). This replaces the former CSS + // injection that hid the teacher-mode toggle: the plugin no longer mutates the + // embedded document, it just passes the supported flag when the per-activity + // setting opts in. + exeweb_apply_teacher_mode_param($moodleurl, $exeweb); + // Let the module handle the display. $PAGE->activityheader->set_description(exeweb_get_intro($exeweb, $cm)); exeweb_print_header($exeweb, $cm, $course); - if (!exeweb_is_teacher_mode_visible($exeweb)) { - exeweb_require_teacher_mode_hider_for_iframe('exewebobject'); - } - echo $PAGE->get_renderer('mod_exeweb')->generate_embed_general($cm, $moodleurl, $title, $clicktoopen); echo $OUTPUT->footer(); @@ -68,7 +72,14 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { } /** - * Check whether teacher mode toggler should be visible for this activity. + * Whether this activity opts in to revealing eXeLearning's teacher-only content. + * + * This is the per-activity "teachermodevisible" setting stored in displayoptions. + * When on, exeweb_display_embed() appends the package's own ?exe-teacher=1 URL + * parameter so the in-package teacher-layer selector is available to viewers (it + * replaces the former parent-side CSS injection). The default when the key is absent + * is false, matching the form default and the "hidden by default, opt in to reveal" + * principle from upstream exelearning#1772 (legacy rows therefore keep it hidden). * * @param stdClass $exeweb * @return bool @@ -76,43 +87,52 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { function exeweb_is_teacher_mode_visible($exeweb) { $options = empty($exeweb->displayoptions) ? [] : (array) unserialize_array($exeweb->displayoptions); if (!array_key_exists('teachermodevisible', $options)) { - return true; + return false; } return !empty($options['teachermodevisible']); } /** - * Inject CSS into the embedded iframe to hide the teacher mode toggler. + * Append eXeLearning's ?exe-teacher=1 reveal parameter to a content URL when the + * activity opts in (teachermodevisible). * - * @param string $iframeid - * @return void + * Centralizes the rule so every display mode — embedded iframe, popup, new window + * and the direct file redirect — reveals the in-package teacher-layer selector + * consistently. Without this, only the embedded iframe carried the flag and the + * other modes silently dropped it. + * + * @param moodle_url $url content/pluginfile URL to flag in place + * @param stdClass $exeweb + * @return moodle_url the same URL, with exe-teacher=1 added when reveal is on */ -function exeweb_require_teacher_mode_hider_for_iframe(string $iframeid): void { - global $PAGE; - - $iframeidjson = json_encode($iframeid); - $cssjson = json_encode('#teacher-mode-toggler-wrapper { visibility: hidden !important; }'); - - $js = "(function(){" - . "var iframe=document.getElementById(" . $iframeidjson . ");" - . "if(!iframe){return;}" - . "var css=" . $cssjson . ";" - . "var inject=function(){try{if(!iframe.contentDocument){return;}" - . "var d=iframe.contentDocument;var st=d.createElement('style');st.textContent=css;" - . "(d.head||d.documentElement).appendChild(st);}catch(e){}};" - . "iframe.addEventListener('load', inject);inject();" - . "})();"; - - $PAGE->requires->js_init_code($js); +function exeweb_apply_teacher_mode_param(moodle_url $url, $exeweb): moodle_url { + if (exeweb_is_teacher_mode_visible($exeweb)) { + $url->param('exe-teacher', '1'); + } + return $url; } -function exeweb_get_clicktoopen($file, $revision, $extra='') { +/** + * Build the "click to open" link for the package entry file. + * + * @param stored_file $file + * @param int $revision + * @param string $extra extra HTML attributes for the anchor + * @param stdClass|null $exeweb activity record; when provided the link honours the + * teacher-mode reveal setting by appending ?exe-teacher=1 + * @return string + */ +function exeweb_get_clicktoopen($file, $revision, $extra='', $exeweb = null) { global $CFG; $filename = $file->get_filename(); $fullurl = moodle_url::make_pluginfile_url($file->get_contextid(), 'mod_exeweb', 'content', $revision, $file->get_filepath(), $filename); + if ($exeweb !== null) { + exeweb_apply_teacher_mode_param($fullurl, $exeweb); + } + $string = get_string('clicktoopen2', 'mod_exeweb', "$filename"); return $string; @@ -143,23 +163,24 @@ function exeweb_print_workaround($exeweb, $cm, $course, $file) { case RESOURCELIB_DISPLAY_POPUP: $fullurl = moodle_url::make_pluginfile_url($file->get_contextid(), 'mod_exeweb', 'content', $exeweb->revision, $file->get_filepath(), $file->get_filename()); + exeweb_apply_teacher_mode_param($fullurl, $exeweb); $options = empty($exeweb->displayoptions) ? [] : (array) unserialize_array($exeweb->displayoptions); $width = empty($options['popupwidth']) ? 620 : $options['popupwidth']; $height = empty($options['popupheight']) ? 450 : $options['popupheight']; $wh = "width=$width,height=$height,toolbar=no,location=no,menubar=no,copyhistory=no," . "status=no,directories=no,scrollbars=yes,resizable=yes"; $extra = "onclick=\"window.open('$fullurl', '', '$wh'); return false;\""; - echo exeweb_get_clicktoopen($file, $exeweb->revision, $extra); + echo exeweb_get_clicktoopen($file, $exeweb->revision, $extra, $exeweb); break; case RESOURCELIB_DISPLAY_NEW: $extra = 'onclick="this.target=\'_blank\'"'; - echo exeweb_get_clicktoopen($file, $exeweb->revision, $extra); + echo exeweb_get_clicktoopen($file, $exeweb->revision, $extra, $exeweb); break; case RESOURCELIB_DISPLAY_OPEN: default: - echo exeweb_get_clicktoopen($file, $exeweb->revision); + echo exeweb_get_clicktoopen($file, $exeweb->revision, '', $exeweb); break; } echo ''; diff --git a/mod_form.php b/mod_form.php index eaac867..082cc96 100644 --- a/mod_form.php +++ b/mod_form.php @@ -175,7 +175,7 @@ public function definition() { $mform->addElement('advcheckbox', 'teachermodevisible', get_string('teachermodevisible', 'exeweb')); $mform->addHelpButton('teachermodevisible', 'teachermodevisible', 'exeweb'); - $mform->setDefault('teachermodevisible', 1); + $mform->setDefault('teachermodevisible', 0); $options = ['0' => get_string('none'), '1' => get_string('allfiles'), '2' => get_string('htmlfilesonly'), ]; @@ -239,7 +239,7 @@ public function data_preprocessing(&$defaultvalues) { if (array_key_exists('teachermodevisible', $displayoptions)) { $defaultvalues['teachermodevisible'] = (int) $displayoptions['teachermodevisible']; } else { - $defaultvalues['teachermodevisible'] = 1; + $defaultvalues['teachermodevisible'] = 0; } } } diff --git a/tests/behat/teacher_mode.feature b/tests/behat/teacher_mode.feature new file mode 100644 index 0000000..58e5fbb --- /dev/null +++ b/tests/behat/teacher_mode.feature @@ -0,0 +1,65 @@ +@mod @mod_exeweb +Feature: Show the eXeLearning teacher-layer selector via the exe-teacher URL parameter + In order to let viewers show or hide the teacher-only layer of an embedded eXeLearning resource + As a teacher configuring the activity + I need the embedded resource to expose its teacher-layer selector when the per-activity setting is on + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | One | teacher1@example.com | + | student1 | Student | One | student1@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + + # The iframe src is server-rendered, so these scenarios assert on it directly + # without @javascript. eXeLearning hides teacher-only content by default and exposes a + # selector to show it via the package's own ?exe-teacher=1 URL parameter (upstream + # exelearning#1772); the plugin appends it whenever the per-activity "Show teacher + # layer selector" setting is on — for any viewer. + # Embed mode (display=1) renders the package in an iframe; its server-rendered src must + # carry the parameter when the setting is on, for any viewer. + Scenario: A teacher sees the exe-teacher parameter when the setting is on + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Teacher reveal | 1 | 1 | + And I am on the "Teacher reveal" "exeweb activity" page logged in as teacher1 + Then the "src" attribute of "iframe#exewebobject" "css_element" should contain "exe-teacher=1" + + Scenario: A teacher does not see the exe-teacher parameter when the reveal is off + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Teacher noreveal | 1 | 0 | + And I am on the "Teacher noreveal" "exeweb activity" page logged in as teacher1 + Then the "src" attribute of "iframe#exewebobject" "css_element" should not contain "exe-teacher" + + Scenario: A student also sees the exe-teacher parameter when the setting is on + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Teacher student vw | 1 | 1 | + And I am on the "Teacher student vw" "exeweb activity" page logged in as student1 + Then the "src" attribute of "iframe#exewebobject" "css_element" should contain "exe-teacher=1" + + # Popup mode (display=6) does not embed an iframe: it renders a server-side "click to open" + # link on the workaround page, whose href and window.open() URL must also carry the + # parameter — otherwise the package opens without the teacher-layer selector in the + # non-embed modes, the regression reported on PR #65. + Scenario: The popup link and window.open URL carry the exe-teacher parameter when the setting is on + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Popup reveal | 6 | 1 | + And I am on the "Popup reveal" "exeweb activity" page logged in as teacher1 + Then the "href" attribute of ".exewebworkaround a" "css_element" should contain "exe-teacher=1" + And the "onclick" attribute of ".exewebworkaround a" "css_element" should contain "exe-teacher=1" + + Scenario: The popup link omits the exe-teacher parameter when the reveal is off + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Popup noreveal | 6 | 0 | + And I am on the "Popup noreveal" "exeweb activity" page logged in as teacher1 + Then the "href" attribute of ".exewebworkaround a" "css_element" should not contain "exe-teacher" diff --git a/tests/locallib_test.php b/tests/locallib_test.php new file mode 100644 index 0000000..8d05623 --- /dev/null +++ b/tests/locallib_test.php @@ -0,0 +1,111 @@ +. + +/** + * Unit tests for mod_exeweb locallib. + * + * @package mod_exeweb + * @category test + * @copyright 2026 ATE (Área de Tecnología Educativa) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_exeweb; + +/** + * Unit tests for mod_exeweb locallib. + * + * @package mod_exeweb + * @category test + * @copyright 2026 ATE (Área de Tecnología Educativa) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers ::exeweb_is_teacher_mode_visible + * @covers ::exeweb_apply_teacher_mode_param + * @covers ::exeweb_get_clicktoopen + */ +class locallib_test extends \advanced_testcase { + + /** + * Loads locallib (and lib) before the test case runs. + * @return void + */ + public static function setUpBeforeClass(): void { + global $CFG; + require_once($CFG->dirroot . '/mod/exeweb/locallib.php'); + } + + /** + * exeweb_is_teacher_mode_visible() defaults to false when the option is absent and + * otherwise mirrors the stored per-activity flag. + * @return void + */ + public function test_is_teacher_mode_visible() { + // Absent option -> default false (matches the form default and the opt-in principle). + $this->assertFalse(exeweb_is_teacher_mode_visible((object) [])); + $this->assertFalse(exeweb_is_teacher_mode_visible((object) ['displayoptions' => ''])); + // Stored 1 -> true, stored 0 -> false. + $this->assertTrue( + exeweb_is_teacher_mode_visible((object) ['displayoptions' => serialize(['teachermodevisible' => 1])]) + ); + $this->assertFalse( + exeweb_is_teacher_mode_visible((object) ['displayoptions' => serialize(['teachermodevisible' => 0])]) + ); + } + + /** + * exeweb_apply_teacher_mode_param() adds ?exe-teacher=1 only when the activity opts in. + * This is the centralized rule shared by every display mode (embed, popup, new, redirect). + * @return void + */ + public function test_apply_teacher_mode_param() { + $base = new \moodle_url('/pluginfile.php/1/mod_exeweb/content/1/index.html'); + + // Reveal on -> param appended. + $on = (object) ['displayoptions' => serialize(['teachermodevisible' => 1])]; + $this->assertStringContainsString( + 'exe-teacher=1', exeweb_apply_teacher_mode_param(clone $base, $on)->out(false)); + + // Reveal off or absent -> param not added. + $off = (object) ['displayoptions' => serialize(['teachermodevisible' => 0])]; + $this->assertStringNotContainsString( + 'exe-teacher', exeweb_apply_teacher_mode_param(clone $base, $off)->out(false)); + $this->assertStringNotContainsString( + 'exe-teacher', exeweb_apply_teacher_mode_param(clone $base, (object) [])->out(false)); + } + + /** + * exeweb_get_clicktoopen() embeds ?exe-teacher=1 in the click-to-open link used by the + * popup and open display modes when the reveal setting is on, and omits it when off. + * This is the link path that previously dropped the parameter for non-embed modes. + * @return void + */ + public function test_clicktoopen_contains_teacher_param() { + $this->resetAfterTest(); + + $fs = get_file_storage(); + $file = $fs->create_file_from_string([ + 'contextid' => \context_system::instance()->id, 'component' => 'mod_exeweb', + 'filearea' => 'content', 'itemid' => 1, 'filepath' => '/', 'filename' => 'index.html', + ], ''); + + // Reveal on -> the link carries the parameter. + $on = (object) ['displayoptions' => serialize(['teachermodevisible' => 1])]; + $this->assertStringContainsString('exe-teacher=1', exeweb_get_clicktoopen($file, 1, '', $on)); + + // Reveal off -> no parameter. + $off = (object) ['displayoptions' => serialize(['teachermodevisible' => 0])]; + $this->assertStringNotContainsString('exe-teacher', exeweb_get_clicktoopen($file, 1, '', $off)); + } +} diff --git a/view.php b/view.php index 909ccbb..0e1cd0b 100644 --- a/view.php +++ b/view.php @@ -81,6 +81,9 @@ // this redirect trick solves caching problems when tracking views ;-) . $path = '/'.$context->id.'/mod_exeweb/content/'.$exeweb->revision.$file->get_filepath().$file->get_filename(); $fullurl = moodle_url::make_file_url('/pluginfile.php', $path); + // Reveal eXeLearning's teacher-only content (?exe-teacher=1) when the activity opts in, + // matching the embedded iframe path so every display mode stays consistent. + exeweb_apply_teacher_mode_param($fullurl, $exeweb); redirect($fullurl); }