codeant-ai-for-open-source[bot] commented on PR #36323: URL: https://github.com/apache/superset/pull/36323#issuecomment-3607715044
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-f915f315beeedb4a7adaead3388805140efb2d76a44e08ae1670235db4ac9aabR155-R159'><strong>Possible XSS</strong></a><br>The new tooltip content is injected via `.html()` using `getNameOfRegion(d)` and `format(...)` which may contain unescaped/unsanitized values coming from geo JSON or data. This can open an XSS vector if any of those values can be controlled or contain HTML. Prefer using text-safe APIs or explicit escaping/sanitization.<br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-230098aa7caefe6bb4999da9a8027f8c376e52216e7c28ddf6e2a320f1dfa888R25-R59'><strong>Persistent spies</strong></a><br>Several global jest.spyOn calls are created at module scope (e.g. d3.json, d3.geo.path, d3.geo.mercator, d3.mouse). These spies are not restored after the test file runs and may leak into other tests or interfere with Jest/Node module caching. Consider restoring spies or creating/tearing them down per-test to avoid cross-test pollution.<br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-f915f315beeedb4a7adaead3388805140efb2d76a44e08ae1670235db4ac9aabR93-R93'><strong>Missing styling / deleted CSS</strong></a><br>The PR creates a tooltip DOM node with class `hover-popup` but the file CountryMap.css was deleted. The tooltip relies on CSS for layout, z-index, visibility, pointer-events, etc. Without styles the popup may be invisible, mis-positioned, or be drawn under the map regions and not address the original visual bug.<br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-f915f315beeedb4a7adaead3388805140efb2d76a44e08ae1670235db4ac9aabR149-R166'><strong>Tooltip positioning / clipping</strong></a><br>The tooltip is positioned using `d3.mouse(svg.node())` and absolute top/left offsets. This can cause the popup to be positioned off-screen or under map elements (especially after zoom/translate). The logic does not clamp tooltip position to viewport or account for tooltip size, nor does it append to document.body to avoid container overflow.<br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-f7611a7af88f3617e26f5f6562add24ba45c7a5784d18f03e0ac42c49417cfa6R46-R57'><strong>Tooltip event blocking</strong></a><br>The new `.hover-popup` element is positioned absolutely over the map but does not set `pointer-events`. As currently implemented, the popup will receive mouse events (default `pointer-events: auto`), which can block pointer interactions with the underlying SVG regions (hover, click). Confirm whether the tooltip should be interactive; if not, the CSS should avoid intercepting pointer events.<br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-f7611a7af88f3617e26f5f6562add24ba45c7a5784d18f03e0ac42c49417cfa6R59-R63'><strong>map-layer pointer-events change</strong></a><br>The `.map-layer` rule now explicitly sets `pointer-events: all;`. This may be intentional to ensure regions receive events, but it could cause unexpected behavior in some browsers or interact badly with stacked SVG elements and overlays. Verify this is necessary and doesn't conflict with other pointer-event rules (e.g., `.effect-layer` uses `pointer-events: none`).<br> - [ ] <a href='https://github.com/apache/superset/pull/36323/files#diff-230098aa7caefe6bb4999da9a8027f8c376e52216e7c28ddf6e2a320f1dfa888R47-R56'><strong>Mocking deprecated d3 API</strong></a><br>The test mocks legacy d3 APIs (e.g. `d3.geo.path`, `d3.geo.mercator`, `d3.mouse`). Depending on the d3 version used in the environment, these functions may be absent or behave differently, causing runtime errors or brittle tests. Verify the d3 version and prefer mocking specific module exports or adapt the mocks to match the exact API surface.<br> </td></tr> </table> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
