mikebridge commented on PR #43830: URL: https://github.com/apache/superset/pull/43830#issuecomment-5531355511
Ran a 2-lens internal review (React/TypeScript + a structured PR-style pass) over this diff — both approved, no blocking findings. Folded the actionable notes into `98fdcd1963`: - **Structural same-origin enforcement in `navigateOpenedTab`.** Dropping `noopener` from the `about:blank` placeholder means the claimed tab now carries an opener link. That's safe for the two callers here (both pass relative `/explore/…` and `/dashboard/…` paths), but `assertSafeNavigationUrl` also permits safe absolute URLs, so the same-origin property was caller convention rather than enforced. `navigateOpenedTab` now reuses the opener-connected tab **only** for a same-origin route; an absolute/external URL closes the claimed tab and reopens through the `noopener` fallback — so no destination ever rides the opener chain. Regression test added. - Added a test asserting `openBlankTab` returns `null` on a blocked popup (the signal callers rely on), and made the new test block's `window.open` spy default to a no-op for consistency with the rest of the file. File is at 82/82 tests, changed-file pre-commit clean (oxfmt/oxlint/tsc). -- 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]
