From ee7a61229868aabf9c71a1dd030dc424c365dbbf Mon Sep 17 00:00:00 2001
From: erseco <info@ernesto.es>
Date: Sat, 9 May 2026 18:13:37 +0100
Subject: [PATCH] fix: guard TOC navigation against re-entry from
 closeAll/openAll (#63)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the user clicks a TOC entry, `exescorm_activate_item()` calls
`exescorm_tree_node.closeAll()` and later `openAll()` to collapse and
re-expand the entire YUI TreeView. Those operations mutate node
selection state and YUI fires additional `select` events while the
tree settles, which the `tree.after('select', ...)` handler then
interprets as a fresh navigation request and calls
`exescorm_activate_item()` again with a *different* node (typically a
sibling). The visible symptoms reported in #63 — the iframe loading
twice and "click X, see Y" — line up with that pattern.

Add an `exescorm_navigating` flag set right before the first state
change and cleared after `openAll()`. The select handler returns early
when the flag is set, so only the original user click drives the
navigation.

The fix is defensive: I have not been able to reproduce locally on
Moodle 5.0, but every signal in the report (intermittent, multiple
iframe loads, sibling shown instead of clicked item) points at
re-entry through this code path. If the symptom persists after
upgrading we'll need to capture a console trace inside the failing
session.
---
 module.js | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/module.js b/module.js
index ab4389f..42741d9 100644
--- a/module.js
+++ b/module.js
@@ -86,6 +86,12 @@ M.mod_exescorm.init = function(Y, nav_display, navposition_left, navposition_top
     var exescorm_buttons = [];
     var exescorm_bloody_labelclick = false;
     var exescorm_nav_panel;
+    // Re-entry guard for `tree.after('select', ...)`. `exescorm_activate_item`
+    // collapses + re-expands the whole TreeView via `closeAll()`/`openAll()`,
+    // and YUI fires extra `select` events during that dance. Without this
+    // flag we re-entered the navigation handler with a sibling node and ended
+    // up loading the wrong page in the iframe (issue #63).
+    var exescorm_navigating = false;
 
     Y.use('button', 'dd-plugin', 'panel', 'resize', 'gallery-sm-treeview', function(Y) {
 
@@ -175,6 +181,10 @@ M.mod_exescorm.init = function(Y, nav_display, navposition_left, navposition_top
             // End of - Avoid recursive calls.
 
             exescorm_current_node = node;
+            // Suppress the 'select' re-entry the next .select()/closeAll()/openAll()
+            // calls would otherwise generate (issue #63). Cleared after openAll()
+            // below.
+            exescorm_navigating = true;
             if (!exescorm_current_node.state.selected) {
                 exescorm_current_node.select();
             }
@@ -230,6 +240,7 @@ M.mod_exescorm.init = function(Y, nav_display, navposition_left, navposition_top
                 exescorm_fixnav();
             }
             exescorm_tree_node.openAll();
+            exescorm_navigating = false;
         };
 
         mod_exescorm_activate_item = exescorm_activate_item;
@@ -665,6 +676,12 @@ M.mod_exescorm.init = function(Y, nav_display, navposition_left, navposition_top
         exescorm_tree_node = tree;
         // Trigger after instead of on, avoid recursive calls.
         tree.after('select', function(e) {
+            // Skip select events triggered by programmatic state changes inside
+            // exescorm_activate_item (closeAll/openAll). See exescorm_navigating
+            // declaration for context.
+            if (exescorm_navigating) {
+                return;
+            }
             var node = e.node;
             if (node.title == '' || node.title == null) {
                 return; //this item has no navigation