From 1a3d0ad9ae690a56528c213958547745193cb36a Mon Sep 17 00:00:00 2001 From: erseco Date: Fri, 19 Jun 2026 06:09:16 +0100 Subject: [PATCH 1/4] Reveal eXeLearning teacher content via ?exe-teacher URL param instead of CSS injection eXeLearning core now hides teacher-only content by default and reveals it via the package's own ?exe-teacher=1 URL parameter (upstream exelearning#1772). Consume that contract here and retire the plugin's parent-side teacher-mode CSS injection, which coupled the plugin to the package internals. exeweb_display_embed() now appends ?exe-teacher=1 to the iframe content URL only for users who can manage the activity AND when the per-activity setting opts in; students never receive the parameter and always see the student view. The decision is a pure, unit-tested helper, exeweb_should_reveal_teacher_content(). Removed: - exeweb_require_teacher_mode_hider_for_iframe() and its call in exeweb_display_embed() (injected CSS into the embedded iframe contentDocument to hide #teacher-mode-toggler-wrapper). The teachermodevisible field is kept (mod_form default 1, lib.php displayoptions) and repurposed: it now gates whether teachers see eXeLearning's teacher content revealed in the embedded view (combined with the manageactivities capability). Updated the lang help (en + the hand-maintained es) to the new meaning; the exeweb_is_teacher_mode_visible() docblock too. Tests: - tests/locallib_test.php covers the new helper across the truth table and exeweb_is_teacher_mode_visible() default/stored behaviour. - tests/behat/teacher_mode.feature adds server-rendered scenarios asserting the iframe src contains/omits exe-teacher for teacher (on/off) and student. --- lang/en/exeweb.php | 4 +- lang/es/exeweb.php | 4 +- locallib.php | 61 ++++++++++++++---------- tests/behat/teacher_mode.feature | 44 +++++++++++++++++ tests/locallib_test.php | 81 ++++++++++++++++++++++++++++++++ 5 files changed, 165 insertions(+), 29 deletions(-) create mode 100644 tests/behat/teacher_mode.feature create mode 100644 tests/locallib_test.php diff --git a/lang/en/exeweb.php b/lang/en/exeweb.php index 333b72b..47cc1cb 100644 --- a/lang/en/exeweb.php +++ b/lang/en/exeweb.php @@ -172,8 +172,8 @@ $string['typeexewebedit'] = 'Edit with eXeLearning'; $string['typelocal'] = 'Uploaded package'; -$string['teachermodevisible'] = 'Show teacher layer selector'; -$string['teachermodevisible_help'] = 'If disabled, the teacher layer selector inside the embedded resource is hidden.'; +$string['teachermodevisible'] = 'Reveal eXeLearning teacher content to teachers'; +$string['teachermodevisible_help'] = 'eXeLearning packages can mark content as "teacher only", which the package hides by default. When this setting is enabled, teachers (users who can manage the activity) see that teacher content revealed in the embedded resource; the plugin asks the package to show it via its supported URL parameter. Students never see teacher content regardless of this setting. When disabled, even teachers see the student view.'; // Embedded editor management. $string['manageembeddededitor'] = 'Manage embedded editor'; diff --git a/lang/es/exeweb.php b/lang/es/exeweb.php index d0a44ac..c67bea7 100644 --- a/lang/es/exeweb.php +++ b/lang/es/exeweb.php @@ -246,8 +246,8 @@ $string['typeexewebedit'] = 'Editar con eXeLearning'; $string['typelocal'] = 'Paquete subido'; -$string['teachermodevisible'] = 'Mostrar el selector de capa docente'; -$string['teachermodevisible_help'] = 'Si se desactiva, se ocultará el selector de capa docente dentro del recurso incrustado.'; +$string['teachermodevisible'] = 'Mostrar el contenido docente de eXeLearning al profesorado'; +$string['teachermodevisible_help'] = 'Los paquetes de eXeLearning pueden marcar contenido como "solo para el profesorado", que el paquete oculta de forma predeterminada. Cuando esta opción está activada, el profesorado (las personas que pueden gestionar la actividad) ve ese contenido docente revelado en el recurso incrustado; el plugin solicita al paquete que lo muestre mediante su parámetro de URL admitido. El alumnado nunca ve el contenido docente, independientemente de esta opción. Cuando está desactivada, incluso el profesorado ve la vista del alumnado.'; $string['editoruploadmissingfile'] = 'No se ha subido ningún archivo ZIP del editor.'; $string['editoruploadfailed'] = 'No se pudo subir el paquete del editor: {$a}'; diff --git a/locallib.php b/locallib.php index a1c9899..3a6221b 100644 --- a/locallib.php +++ b/locallib.php @@ -52,15 +52,23 @@ 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. The reveal is granted only + // to users who can manage the activity AND when the per-activity setting opts in; + // students never receive the parameter, so they always see the student view. + $canpreview = has_capability('moodle/course:manageactivities', $context); + if (exeweb_should_reveal_teacher_content($canpreview, exeweb_is_teacher_mode_visible($exeweb))) { + $moodleurl->param('exe-teacher', '1'); + } + // 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 +76,13 @@ 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. + * It used to gate the parent-side CSS that hid the in-package teacher-mode toggle; + * it now gates whether the plugin asks the package to reveal teacher content via the + * ?exe-teacher=1 URL parameter (see exeweb_should_reveal_teacher_content()). The + * default when the key is absent is true, matching the form default and legacy rows. * * @param stdClass $exeweb * @return bool @@ -82,28 +96,25 @@ function exeweb_is_teacher_mode_visible($exeweb) { } /** - * Inject CSS into the embedded iframe to hide the teacher mode toggler. + * Whether to reveal eXeLearning's teacher-only content in the embedded view. * - * @param string $iframeid - * @return void + * eXeLearning packages hide teacher-marked content by default and opt in to reveal + * it via the ?exe-teacher=1 URL parameter (upstream exelearning#1772). + * exeweb_display_embed() appends that parameter to the iframe content URL when this + * returns true. It does so only for users who can manage the activity AND when the + * per-activity setting opts in, so a student never receives the parameter and always + * sees the student view. This replaces the former parent-side CSS injection that hid + * the in-package teacher-mode toggle. + * + * Extracted as a pure function so the decision is unit-testable without rendering the + * embedded view (project philosophy: extract testable pure functions). + * + * @param bool $canpreview Whether the user can manage the activity (teacher/editing-teacher). + * @param bool $teachermodevisible Whether the per-activity setting opts in to revealing teacher content. + * @return bool True when the iframe should request the teacher view. */ -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_should_reveal_teacher_content(bool $canpreview, bool $teachermodevisible): bool { + return $canpreview && $teachermodevisible; } function exeweb_get_clicktoopen($file, $revision, $extra='') { diff --git a/tests/behat/teacher_mode.feature b/tests/behat/teacher_mode.feature new file mode 100644 index 0000000..91806a2 --- /dev/null +++ b/tests/behat/teacher_mode.feature @@ -0,0 +1,44 @@ +@mod @mod_exeweb +Feature: Reveal eXeLearning teacher content via the exe-teacher URL parameter + In order to keep teacher-only content hidden from students + As a teacher + I need the embedded resource to request the teacher view only for teachers who opted in + + 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 reveals + # it via the package's own ?exe-teacher=1 URL parameter (upstream exelearning#1772); + # the plugin appends it only for users who can manage the activity AND when the + # per-activity "Reveal eXeLearning teacher content to teachers" setting is on. + Scenario: A teacher sees the exe-teacher parameter when teacher content is revealed + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Teacher reveal | 5 | 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 | 5 | 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 never sees the exe-teacher parameter even when the reveal is on + Given the following "activities" exist: + | activity | course | name | display | teachermodevisible | + | exeweb | C1 | Teacher student vw | 5 | 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 not contain "exe-teacher" diff --git a/tests/locallib_test.php b/tests/locallib_test.php new file mode 100644 index 0000000..c58b06f --- /dev/null +++ b/tests/locallib_test.php @@ -0,0 +1,81 @@ +. + +/** + * 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_should_reveal_teacher_content + * @covers ::exeweb_is_teacher_mode_visible + */ +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_should_reveal_teacher_content() reveals teacher content only when the + * user can manage the activity AND the per-activity setting opts in. A student + * (cannot manage) never gets the reveal regardless of the setting. + * @return void + */ + public function test_should_reveal_teacher_content() { + // Teacher (can manage) + setting on -> reveal. + $this->assertTrue(exeweb_should_reveal_teacher_content(true, true)); + // Teacher but setting off -> student view even for the teacher. + $this->assertFalse(exeweb_should_reveal_teacher_content(true, false)); + // Student (cannot manage) is never revealed, even if the setting is on. + $this->assertFalse(exeweb_should_reveal_teacher_content(false, true)); + $this->assertFalse(exeweb_should_reveal_teacher_content(false, false)); + } + + /** + * exeweb_is_teacher_mode_visible() defaults to true 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 true (matches the form default and legacy rows). + $this->assertTrue(exeweb_is_teacher_mode_visible((object) [])); + $this->assertTrue(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])]) + ); + } +} From 65e821ea3832e8c0c1760c263ca56aef88e08768 Mon Sep 17 00:00:00 2001 From: erseco Date: Fri, 19 Jun 2026 07:53:57 +0100 Subject: [PATCH 2/4] fix(teacher-mode): gate the teacher-layer selector on the setting alone Drop the capability gate. The per-activity teachermodevisible setting alone controls whether the package's own ?exe-teacher=1 selector is offered, and it is offered to every viewer (students included) when on. Remove the now-trivial exeweb_should_reveal_teacher_content() helper and its unit test, restore the original "Show teacher layer selector" translations, and update the Behat scenario (a student also gets the parameter when the setting is on). --- lang/en/exeweb.php | 4 ++-- lang/es/exeweb.php | 4 ++-- locallib.php | 38 ++++++-------------------------- tests/behat/teacher_mode.feature | 22 +++++++++--------- tests/locallib_test.php | 17 -------------- 5 files changed, 22 insertions(+), 63 deletions(-) diff --git a/lang/en/exeweb.php b/lang/en/exeweb.php index 47cc1cb..333b72b 100644 --- a/lang/en/exeweb.php +++ b/lang/en/exeweb.php @@ -172,8 +172,8 @@ $string['typeexewebedit'] = 'Edit with eXeLearning'; $string['typelocal'] = 'Uploaded package'; -$string['teachermodevisible'] = 'Reveal eXeLearning teacher content to teachers'; -$string['teachermodevisible_help'] = 'eXeLearning packages can mark content as "teacher only", which the package hides by default. When this setting is enabled, teachers (users who can manage the activity) see that teacher content revealed in the embedded resource; the plugin asks the package to show it via its supported URL parameter. Students never see teacher content regardless of this setting. When disabled, even teachers see the student view.'; +$string['teachermodevisible'] = 'Show teacher layer selector'; +$string['teachermodevisible_help'] = 'If disabled, the teacher layer selector inside the embedded resource is hidden.'; // Embedded editor management. $string['manageembeddededitor'] = 'Manage embedded editor'; diff --git a/lang/es/exeweb.php b/lang/es/exeweb.php index c67bea7..d0a44ac 100644 --- a/lang/es/exeweb.php +++ b/lang/es/exeweb.php @@ -246,8 +246,8 @@ $string['typeexewebedit'] = 'Editar con eXeLearning'; $string['typelocal'] = 'Paquete subido'; -$string['teachermodevisible'] = 'Mostrar el contenido docente de eXeLearning al profesorado'; -$string['teachermodevisible_help'] = 'Los paquetes de eXeLearning pueden marcar contenido como "solo para el profesorado", que el paquete oculta de forma predeterminada. Cuando esta opción está activada, el profesorado (las personas que pueden gestionar la actividad) ve ese contenido docente revelado en el recurso incrustado; el plugin solicita al paquete que lo muestre mediante su parámetro de URL admitido. El alumnado nunca ve el contenido docente, independientemente de esta opción. Cuando está desactivada, incluso el profesorado ve la vista del alumnado.'; +$string['teachermodevisible'] = 'Mostrar el selector de capa docente'; +$string['teachermodevisible_help'] = 'Si se desactiva, se ocultará el selector de capa docente dentro del recurso incrustado.'; $string['editoruploadmissingfile'] = 'No se ha subido ningún archivo ZIP del editor.'; $string['editoruploadfailed'] = 'No se pudo subir el paquete del editor: {$a}'; diff --git a/locallib.php b/locallib.php index 3a6221b..38064a3 100644 --- a/locallib.php +++ b/locallib.php @@ -56,11 +56,9 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { // (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. The reveal is granted only - // to users who can manage the activity AND when the per-activity setting opts in; - // students never receive the parameter, so they always see the student view. - $canpreview = has_capability('moodle/course:manageactivities', $context); - if (exeweb_should_reveal_teacher_content($canpreview, exeweb_is_teacher_mode_visible($exeweb))) { + // embedded document, it just passes the supported flag when the per-activity + // setting opts in. + if (exeweb_is_teacher_mode_visible($exeweb)) { $moodleurl->param('exe-teacher', '1'); } @@ -79,10 +77,10 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { * Whether this activity opts in to revealing eXeLearning's teacher-only content. * * This is the per-activity "teachermodevisible" setting stored in displayoptions. - * It used to gate the parent-side CSS that hid the in-package teacher-mode toggle; - * it now gates whether the plugin asks the package to reveal teacher content via the - * ?exe-teacher=1 URL parameter (see exeweb_should_reveal_teacher_content()). The - * default when the key is absent is true, matching the form default and legacy rows. + * 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 true, matching the form default and legacy rows. * * @param stdClass $exeweb * @return bool @@ -95,28 +93,6 @@ function exeweb_is_teacher_mode_visible($exeweb) { return !empty($options['teachermodevisible']); } -/** - * Whether to reveal eXeLearning's teacher-only content in the embedded view. - * - * eXeLearning packages hide teacher-marked content by default and opt in to reveal - * it via the ?exe-teacher=1 URL parameter (upstream exelearning#1772). - * exeweb_display_embed() appends that parameter to the iframe content URL when this - * returns true. It does so only for users who can manage the activity AND when the - * per-activity setting opts in, so a student never receives the parameter and always - * sees the student view. This replaces the former parent-side CSS injection that hid - * the in-package teacher-mode toggle. - * - * Extracted as a pure function so the decision is unit-testable without rendering the - * embedded view (project philosophy: extract testable pure functions). - * - * @param bool $canpreview Whether the user can manage the activity (teacher/editing-teacher). - * @param bool $teachermodevisible Whether the per-activity setting opts in to revealing teacher content. - * @return bool True when the iframe should request the teacher view. - */ -function exeweb_should_reveal_teacher_content(bool $canpreview, bool $teachermodevisible): bool { - return $canpreview && $teachermodevisible; -} - function exeweb_get_clicktoopen($file, $revision, $extra='') { global $CFG; diff --git a/tests/behat/teacher_mode.feature b/tests/behat/teacher_mode.feature index 91806a2..a2aafb6 100644 --- a/tests/behat/teacher_mode.feature +++ b/tests/behat/teacher_mode.feature @@ -1,8 +1,8 @@ @mod @mod_exeweb -Feature: Reveal eXeLearning teacher content via the exe-teacher URL parameter - In order to keep teacher-only content hidden from students - As a teacher - I need the embedded resource to request the teacher view only for teachers who opted in +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: @@ -18,11 +18,11 @@ Feature: Reveal eXeLearning teacher content via the exe-teacher URL parameter | 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 reveals - # it via the package's own ?exe-teacher=1 URL parameter (upstream exelearning#1772); - # the plugin appends it only for users who can manage the activity AND when the - # per-activity "Reveal eXeLearning teacher content to teachers" setting is on. - Scenario: A teacher sees the exe-teacher parameter when teacher content is revealed + # 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. + 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 | 5 | 1 | @@ -36,9 +36,9 @@ Feature: Reveal eXeLearning teacher content via the exe-teacher URL parameter 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 never sees the exe-teacher parameter even when the reveal is on + 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 | 5 | 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 not contain "exe-teacher" + Then the "src" attribute of "iframe#exewebobject" "css_element" should contain "exe-teacher=1" diff --git a/tests/locallib_test.php b/tests/locallib_test.php index c58b06f..9d8a71b 100644 --- a/tests/locallib_test.php +++ b/tests/locallib_test.php @@ -31,7 +31,6 @@ * @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_should_reveal_teacher_content * @covers ::exeweb_is_teacher_mode_visible */ class locallib_test extends \advanced_testcase { @@ -45,22 +44,6 @@ public static function setUpBeforeClass(): void { require_once($CFG->dirroot . '/mod/exeweb/locallib.php'); } - /** - * exeweb_should_reveal_teacher_content() reveals teacher content only when the - * user can manage the activity AND the per-activity setting opts in. A student - * (cannot manage) never gets the reveal regardless of the setting. - * @return void - */ - public function test_should_reveal_teacher_content() { - // Teacher (can manage) + setting on -> reveal. - $this->assertTrue(exeweb_should_reveal_teacher_content(true, true)); - // Teacher but setting off -> student view even for the teacher. - $this->assertFalse(exeweb_should_reveal_teacher_content(true, false)); - // Student (cannot manage) is never revealed, even if the setting is on. - $this->assertFalse(exeweb_should_reveal_teacher_content(false, true)); - $this->assertFalse(exeweb_should_reveal_teacher_content(false, false)); - } - /** * exeweb_is_teacher_mode_visible() defaults to true when the option is absent and * otherwise mirrors the stored per-activity flag. From efd7e6e837afa81ea850db77bfcfc2c2916feae9 Mon Sep 17 00:00:00 2001 From: erseco Date: Fri, 26 Jun 2026 08:32:45 +0100 Subject: [PATCH 3/4] Default teachermodevisible to OFF (align with #1772 opt-in) --- locallib.php | 5 +++-- mod_form.php | 4 ++-- tests/locallib_test.php | 8 ++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/locallib.php b/locallib.php index 38064a3..a6187cb 100644 --- a/locallib.php +++ b/locallib.php @@ -80,7 +80,8 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { * 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 true, matching the form default and legacy rows. + * 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 @@ -88,7 +89,7 @@ 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']); } 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/locallib_test.php b/tests/locallib_test.php index 9d8a71b..d210d1c 100644 --- a/tests/locallib_test.php +++ b/tests/locallib_test.php @@ -45,14 +45,14 @@ public static function setUpBeforeClass(): void { } /** - * exeweb_is_teacher_mode_visible() defaults to true when the option is absent and + * 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 true (matches the form default and legacy rows). - $this->assertTrue(exeweb_is_teacher_mode_visible((object) [])); - $this->assertTrue(exeweb_is_teacher_mode_visible((object) ['displayoptions' => ''])); + // 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])]) From 912e8dd081f6c62e5eb8cdece9bf0e8c2b7bf015 Mon Sep 17 00:00:00 2001 From: erseco Date: Sun, 28 Jun 2026 17:38:56 +0100 Subject: [PATCH 4/4] fix(teacher-mode): reveal exe-teacher in popup/open/redirect modes, not only embed PR #65 flagged only the embedded-iframe URL with eXeLearning's ?exe-teacher=1 reveal parameter. The popup, open and direct-redirect paths build the package content URL separately and dropped it, so the in-package teacher-layer selector never appeared in those display modes (reported by @ignaciogros). Centralize the rule in exeweb_apply_teacher_mode_param() and apply it at every content-URL build site: - exeweb_display_embed() embed iframe (routed through the helper, unchanged) - exeweb_get_clicktoopen() popup/open links (new optional $exeweb argument) - exeweb_print_workaround() popup window.open() URL - view.php redirect block (student popup/open path) Tests: - locallib_test: unit-test the helper and the click-to-open link - teacher_mode.feature: add popup scenarios and fix the embed scenarios to use the real display value (1 = embed; 5 is open, which renders no iframe) --- locallib.php | 47 +++++++++++++++++++++++++++----- tests/behat/teacher_mode.feature | 27 ++++++++++++++++-- tests/locallib_test.php | 47 ++++++++++++++++++++++++++++++++ view.php | 3 ++ 4 files changed, 114 insertions(+), 10 deletions(-) diff --git a/locallib.php b/locallib.php index a6187cb..7d2889c 100644 --- a/locallib.php +++ b/locallib.php @@ -58,9 +58,7 @@ function exeweb_display_embed($exeweb, $cm, $course, $file) { // 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. - if (exeweb_is_teacher_mode_visible($exeweb)) { - $moodleurl->param('exe-teacher', '1'); - } + exeweb_apply_teacher_mode_param($moodleurl, $exeweb); // Let the module handle the display. $PAGE->activityheader->set_description(exeweb_get_intro($exeweb, $cm)); @@ -94,13 +92,47 @@ function exeweb_is_teacher_mode_visible($exeweb) { return !empty($options['teachermodevisible']); } -function exeweb_get_clicktoopen($file, $revision, $extra='') { +/** + * Append eXeLearning's ?exe-teacher=1 reveal parameter to a content URL when the + * activity opts in (teachermodevisible). + * + * 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_apply_teacher_mode_param(moodle_url $url, $exeweb): moodle_url { + if (exeweb_is_teacher_mode_visible($exeweb)) { + $url->param('exe-teacher', '1'); + } + return $url; +} + +/** + * 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; @@ -131,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/tests/behat/teacher_mode.feature b/tests/behat/teacher_mode.feature index a2aafb6..58e5fbb 100644 --- a/tests/behat/teacher_mode.feature +++ b/tests/behat/teacher_mode.feature @@ -22,23 +22,44 @@ Feature: Show the eXeLearning teacher-layer selector via the exe-teacher URL par # 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 | 5 | 1 | + | 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 | 5 | 0 | + | 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 | 5 | 1 | + | 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 index d210d1c..8d05623 100644 --- a/tests/locallib_test.php +++ b/tests/locallib_test.php @@ -32,6 +32,8 @@ * @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 { @@ -61,4 +63,49 @@ public function test_is_teacher_mode_visible() { 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); }