From 20f17f03aa7b1112f5d9cb155bc44ca487808937 Mon Sep 17 00:00:00 2001
From: Maximilian Haye <m.haye@tu-berlin.de>
Date: Tue, 3 Dec 2024 15:18:28 +0100
Subject: [PATCH] fix: nested shuffle-contents as direct child

---
 classes/question_ui_renderer.php    |  9 +++++----
 tests/question_ui_renderer_test.php | 18 +++++++++++-------
 tests/question_uis/shuffle.xhtml    |  5 +++++
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/classes/question_ui_renderer.php b/classes/question_ui_renderer.php
index 81c20a5..155ec1a 100644
--- a/classes/question_ui_renderer.php
+++ b/classes/question_ui_renderer.php
@@ -174,7 +174,7 @@ private function shuffle_contents(): void {
                 if ($child instanceof DOMElement) {
                     $child = array_pop($childelements);
                     $newelement->appendChild($child);
-                    $this->replace_shuffled_indices($child, $i++);
+                    $this->replace_shuffled_indices($newelement, $child, $i++);
                 } else {
                     $newelement->appendChild($child);
                 }
@@ -188,16 +188,17 @@ private function shuffle_contents(): void {
     /**
      * Among the descendants of `$element`, finds `qpy:shuffled-index` elements and replaces them with `$index`.
      *
-     * @param DOMNode $element
+     * @param DOMElement $container the element which has the currently handled `qpy:shuffle-contents` attribute
+     * @param DOMElement $element the shuffled element whose `qpy:shuffled-index` descendants should be replaced
      * @param int $index
      * @throws coding_exception
      */
-    private function replace_shuffled_indices(DOMNode $element, int $index): void {
+    private function replace_shuffled_indices(DOMElement $container, DOMElement $element, int $index): void {
         /** @var DOMElement $indexelement */
         foreach (iterator_to_array($this->xpath->query('.//qpy:shuffled-index', $element)) as $indexelement) {
             // phpcs:ignore Squiz.ControlStructures.ForLoopDeclaration.SpacingAfterSecond
             for (
-                $ancestor = $indexelement->parentNode; $ancestor !== null && $ancestor !== $element;
+                $ancestor = $indexelement->parentNode; $ancestor !== null && $ancestor !== $container;
                 $ancestor = $ancestor->parentNode
             ) {
                 assert($ancestor instanceof DOMElement);
diff --git a/tests/question_ui_renderer_test.php b/tests/question_ui_renderer_test.php
index 4693c0e..e089bd4 100644
--- a/tests/question_ui_renderer_test.php
+++ b/tests/question_ui_renderer_test.php
@@ -163,15 +163,19 @@ public function test_should_shuffle_correctly_and_replace_indices(): void {
 
         $this->assert_html_string_equals_html_string(<<<EXPECTED
         <div xmlns="http://www.w3.org/1999/xhtml">
-            <span>Element 3, shuffled to a</span>
-            <span>Element 4, shuffled to II</span>
-            <span>Element 2, shuffled to 3</span>
-            <span>Element 1, shuffled to 4</span>
+            <span>Element 1, shuffled to 1</span>
+            <span>Element 3, shuffled to b</span>
             <div>
-                Element 5, shuffled to 5
+                <span>Nested element 1, shuffled to 1</span>
+                <span>Nested element 2, shuffled to 2</span>
+            </div>
+            <span>Element 2, shuffled to 4</span>
+            <span>Element 4, shuffled to V</span>
+            <div>
+                Element 5, shuffled to 6
                 <div>
-                    <span>Nested element 1, shuffled to 1</span>
-                    <span>Nested element 2, shuffled to 2</span>
+                    <span>Nested element 2, shuffled to 1</span>
+                    <span>Nested element 1, shuffled to 2</span>
                 </div>
             </div>
         </div>
diff --git a/tests/question_uis/shuffle.xhtml b/tests/question_uis/shuffle.xhtml
index e533858..19a414d 100644
--- a/tests/question_uis/shuffle.xhtml
+++ b/tests/question_uis/shuffle.xhtml
@@ -27,4 +27,9 @@
             <span>Nested element 2, shuffled to <qpy:shuffled-index/></span>
         </div>
     </div>
+    <div qpy:shuffle-contents="">
+        <!-- Nested shuffle being a direct child is an edge case we need to support. -->
+        <span>Nested element 1, shuffled to <qpy:shuffled-index/></span>
+        <span>Nested element 2, shuffled to <qpy:shuffled-index/></span>
+    </div>
 </div>