fix: prevent ID collisions and namespace global variable

- fix(html): add unique suffix to canvas element IDs
- fix(js): namespace global chart instances as bbsnlyChartJSInstances
- fix(security): prevent potential ID conflicts in loops
- test: update tests to match new ID and namespace patterns

Author: bbsnly

src/Chart.php

@@ -116,8 +116,10 @@ public function toHtml(string $element, Chart $chart = null): string
             $chart = $this;
         }
 
-        $elementId = htmlspecialchars($element, ENT_QUOTES, 'UTF-8');
-        $chartId = 'chart_' . uniqid();
+        $uniqueSuffix = '_' . uniqid();
+        $elementId = htmlspecialchars($element . $uniqueSuffix, ENT_QUOTES, 'UTF-8');
+        $chartId = 'chart' . $uniqueSuffix;
+        $instancesVar = 'bbsnlyChartJSInstances'; // Namespace the global variable
 
         return '<canvas id="' . $elementId . '"></canvas>
         <script>
@@ -131,16 +133,16 @@ public function toHtml(string $element, Chart $chart = null): string
                 const ' . $chartId . ' = new Chart(ctx, ' . $chart->toJson() . ');
 
                 // Store chart instance for potential external access
-                if (typeof window.chartInstances === "undefined") {
-                    window.chartInstances = {};
+                if (typeof window.' . $instancesVar . ' === "undefined") {
+                    window.' . $instancesVar . ' = {};
                 }
-                window.chartInstances["' . $elementId . '"] = ' . $chartId . ';
+                window.' . $instancesVar . '["' . $elementId . '"] = ' . $chartId . ';
 
                 // Cleanup on page unload to prevent memory leaks
                 window.addEventListener("unload", function() {
-                    if (window.chartInstances["' . $elementId . '"]) {
+                    if (window.' . $instancesVar . '["' . $elementId . '"]) {
                         ' . $chartId . '.destroy();
-                        delete window.chartInstances["' . $elementId . '"];
+                        delete window.' . $instancesVar . '["' . $elementId . '"];
                     }
                 });
             })();

tests/ChartTest.php

@@ -355,7 +355,8 @@ public function test_html_special_chars_are_escaped()
         $maliciousId = '"><script>alert("xss")</script>';
         $html = $this->chart->toHtml($maliciousId);
 
-        $this->assertStringContains('id="&quot;&gt;&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt;"', $html);
+        // Update assertion to include unique suffix
+        $this->assertStringContains('id="&quot;&gt;&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt;_', $html);
         $this->assertStringNotContains($maliciousId, $html);
     }
 
@@ -396,9 +397,21 @@ public function test_chart_instances_are_tracked()
     {
         $html = $this->chart->toHtml('test-chart');
 
-        $this->assertStringContains('window.chartInstances', $html);
-        $this->assertStringContains('window.chartInstances["test-chart"]', $html);
-        $this->assertStringContains('delete window.chartInstances', $html);
+        // Update assertions to use correct global variable name
+        $this->assertStringContains('window.bbsnlyChartJSInstances', $html);
+        $this->assertStringContains('bbsnlyChartJSInstances["test-chart_', $html);
+        $this->assertStringContains('delete window.bbsnlyChartJSInstances', $html);
+    }
+
+    public function test_element_ids_are_unique()
+    {
+        $html1 = $this->chart->toHtml('my-chart');
+        $html2 = $this->chart->toHtml('my-chart');
+
+        preg_match('/id="(my-chart_[^"]+)"/', $html1, $matches1);
+        preg_match('/id="(my-chart_[^"]+)"/', $html2, $matches2);
+
+        $this->assertNotEquals($matches1[1], $matches2[1]);
     }
 
     /**