From 5d4358a14938c512d14e3693f79ee78f283d2e8c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:41:25 -0700 Subject: [PATCH 1/6] chore(deps): bump google-auth-library from 11.0.1 to 11.0.2 in /superset-frontend (#43364) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- superset-frontend/package-lock.json | 21 ++++++--------------- superset-frontend/package.json | 2 +- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 357098f954cf..8b91fcd51bdf 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -99,7 +99,7 @@ "geostyler-openlayers-parser": "^5.7.1", "geostyler-style": "11.0.2", "geostyler-wfs-parser": "^3.0.1", - "google-auth-library": "^11.0.1", + "google-auth-library": "^11.0.2", "immer": "^11.1.16", "interweave": "^13.1.1", "jquery": "^4.0.0", @@ -20612,7 +20612,7 @@ "version": "0.8.0", "resolved": "https://registry.npmjs.org/expect-playwright/-/expect-playwright-0.8.0.tgz", "integrity": "sha512-+kn8561vHAY+dt+0gMqqj1oY+g5xWrsuGMk4QGxotT2WS545nVqqjs37z6hrYfIuucwqthzwJfCJUEYqixyljg==", - "deprecated": "\u26a0\ufe0f The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.", + "deprecated": "⚠️ The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.", "dev": true, "license": "MIT" }, @@ -22783,9 +22783,9 @@ "license": "MIT" }, "node_modules/google-auth-library": { - "version": "11.0.1", - "resolved": "https://registry.npmjs.org/google-auth-library/-/google-auth-library-11.0.1.tgz", - "integrity": "sha512-ZqfaYduu9ASUaFuUk5dF9g9QvufdhhSj7jFiEnCrTQcH57sFPKYetM0iU4dcKkQk6CqC1xpSrVr5uQ9NhqjNOg==", + "version": "11.0.2", + "resolved": "https://registry.npmjs.org/google-auth-library/-/google-auth-library-11.0.2.tgz", + "integrity": "sha512-vzpgPutxrghPsnjrjpzLX2bdv8IOL719Rh0oEjGnQu8YCIbnbMuTTQ5zU9LcKvLdOPgCxBwppbvnhgW90Qna5Q==", "license": "Apache-2.0", "dependencies": { "base64-js": "^1.3.0", @@ -26023,7 +26023,7 @@ "version": "0.4.0", "resolved": "https://registry.npmjs.org/jest-process-manager/-/jest-process-manager-0.4.0.tgz", "integrity": "sha512-80Y6snDyb0p8GG83pDxGI/kQzwVTkCxc7ep5FPe/F6JYdvRDhwr6RzRmPSP7SEwuLhxo80lBS/NqOdUIbHIfhw==", - "deprecated": "\u26a0\ufe0f The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.", + "deprecated": "⚠️ The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.", "dev": true, "license": "MIT", "dependencies": { @@ -43073,15 +43073,6 @@ "node": ">=12" } }, - "packages/superset-ui-core/node_modules/dompurify": { - "version": "3.4.13", - "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz", - "integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==", - "license": "(MPL-2.0 OR Apache-2.0)", - "optionalDependencies": { - "@types/trusted-types": "^2.0.7" - } - }, "packages/superset-ui-core/node_modules/react-ace": { "version": "14.0.1", "resolved": "https://registry.npmjs.org/react-ace/-/react-ace-14.0.1.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 26e253ec6587..be48a33313f6 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -176,7 +176,7 @@ "geostyler-openlayers-parser": "^5.7.1", "geostyler-style": "11.0.2", "geostyler-wfs-parser": "^3.0.1", - "google-auth-library": "^11.0.1", + "google-auth-library": "^11.0.2", "immer": "^11.1.16", "interweave": "^13.1.1", "jquery": "^4.0.0", From 80bba12d0fc6dc3570824442b0174f612ba05a4a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:41:38 -0700 Subject: [PATCH 2/6] chore(deps-dev): bump globals from 17.10.0 to 17.11.0 in /docs (#43363) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- docs/package.json | 2 +- docs/yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/package.json b/docs/package.json index 8a5f32d732f6..cbd50bdd617e 100644 --- a/docs/package.json +++ b/docs/package.json @@ -93,7 +93,7 @@ "@typescript-eslint/parser": "^8.67.0", "eslint": "^9.39.2", "eslint-plugin-react": "^7.37.5", - "globals": "^17.10.0", + "globals": "^17.11.0", "oxfmt": "^0.63.0", "typescript": "~6.0.3", "typescript-eslint": "^8.67.0", diff --git a/docs/yarn.lock b/docs/yarn.lock index d61a22c4410b..6c22e5ee02bc 100644 --- a/docs/yarn.lock +++ b/docs/yarn.lock @@ -9174,10 +9174,10 @@ globals@^14.0.0: resolved "https://registry.yarnpkg.com/globals/-/globals-14.0.0.tgz#898d7413c29babcf6bafe56fcadded858ada724e" integrity sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ== -globals@^17.10.0: - version "17.10.0" - resolved "https://registry.yarnpkg.com/globals/-/globals-17.10.0.tgz#f9dbd847ae99e236f98b13095e2426ac3b25a45c" - integrity sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA== +globals@^17.11.0: + version "17.11.0" + resolved "https://registry.yarnpkg.com/globals/-/globals-17.11.0.tgz#d643485bb30220d7751e511cf4f68c73d3870d87" + integrity sha512-Z2I8hM+PbJDXQDq3Icgpzv+mPdwr68iZUU9d5WW4FuXfDUQfkZaZuvjMv42/5crNyw154+9+VWXbYrUgDXbxNw== globalthis@^1.0.4: version "1.0.4" From b288db91f95a7319333e76182c8bac50578b47e7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:41:43 -0700 Subject: [PATCH 3/6] chore(deps): bump github/codeql-action/init from 4.37.6 to 4.37.7 (#43362) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/codeql-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index bf2d7e2f1c79..bb3b41c4f890 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -67,7 +67,7 @@ jobs: # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4.37.6 + uses: github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7 with: languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. From 8b67fb4d308547fc01fccd5516e21cf0e08b700f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:41:49 -0700 Subject: [PATCH 4/6] chore(deps-dev): bump globals from 17.10.0 to 17.11.0 in /superset-websocket (#43360) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- superset-websocket/package-lock.json | 8 ++++---- superset-websocket/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset-websocket/package-lock.json b/superset-websocket/package-lock.json index 0cd1f07701a2..1393d82e94e0 100644 --- a/superset-websocket/package-lock.json +++ b/superset-websocket/package-lock.json @@ -28,7 +28,7 @@ "@typescript-eslint/parser": "^8.67.0", "eslint": "^10.8.1", "eslint-config-prettier": "^10.1.8", - "globals": "^17.10.0", + "globals": "^17.11.0", "oxfmt": "^0.63.0", "tscw-config": "^1.1.2", "typescript": "^6.0.3", @@ -2053,9 +2053,9 @@ } }, "node_modules/globals": { - "version": "17.10.0", - "resolved": "https://registry.npmjs.org/globals/-/globals-17.10.0.tgz", - "integrity": "sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA==", + "version": "17.11.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-17.11.0.tgz", + "integrity": "sha512-Z2I8hM+PbJDXQDq3Icgpzv+mPdwr68iZUU9d5WW4FuXfDUQfkZaZuvjMv42/5crNyw154+9+VWXbYrUgDXbxNw==", "dev": true, "license": "MIT", "engines": { diff --git a/superset-websocket/package.json b/superset-websocket/package.json index b8349ad5aa20..9875e1430ee1 100644 --- a/superset-websocket/package.json +++ b/superset-websocket/package.json @@ -36,7 +36,7 @@ "@typescript-eslint/parser": "^8.67.0", "eslint": "^10.8.1", "eslint-config-prettier": "^10.1.8", - "globals": "^17.10.0", + "globals": "^17.11.0", "oxfmt": "^0.63.0", "tscw-config": "^1.1.2", "typescript": "^6.0.3", From fdd3945dded56d1ae852aac92c2595c6c92fded6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:58:31 +0300 Subject: [PATCH 5/6] chore(deps): bump github/codeql-action/analyze from 4.37.6 to 4.37.7 (#43361) Signed-off-by: dependabot[bot] --- .github/workflows/codeql-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index bb3b41c4f890..fa9337a2da90 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -78,6 +78,6 @@ jobs: # queries: security-extended,security-and-quality - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@5595ccaf912efad79be6eef63a5619ff05969be3 # v4.37.6 + uses: github/codeql-action/analyze@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7 with: category: "/language:${{matrix.language}}" From e7dccd44a7c212739147155548e689e9d6b3408f Mon Sep 17 00:00:00 2001 From: Mafi Date: Thu, 20 Aug 2026 23:10:30 +1000 Subject: [PATCH 6/6] fix(reports): prevent blank/partial report PDFs from virtualized charts (#43348) Co-authored-by: Matt Fitzgerald Co-authored-by: Claude Opus 4.8 Co-authored-by: Amin Ghadersohi --- superset/utils/screenshot_utils.py | 68 ++++++++- superset/utils/webdriver.py | 48 +++++- tests/unit_tests/utils/webdriver_test.py | 182 +++++++++++++++++++++++ 3 files changed, 285 insertions(+), 13 deletions(-) diff --git a/superset/utils/screenshot_utils.py b/superset/utils/screenshot_utils.py index 365ff5b56246..e9c1230a791f 100644 --- a/superset/utils/screenshot_utils.py +++ b/superset/utils/screenshot_utils.py @@ -159,16 +159,49 @@ class TiledScreenshotBudgetExceededError(ScreenshotTaskBudgetExceededError): f"{ALERT_SELECTOR}, {EMPTY_SELECTOR}, {MISSING_CHART_SELECTOR}" ) -# Shared body for holder readiness and timeout diagnostics. A holder is ready -# only after a terminal marker appears and its loading marker disappears. -UNREADY_CHART_HOLDERS_JS_BODY = f""" - const holders = document.querySelectorAll('{CHART_HOLDER_SELECTOR}'); - const unready = []; - for (const holder of holders) {{ +# Runtime contract with the dashboard frontend. Dispatching this window event +# forces every DashboardVirtualization row to render regardless of whether it +# intersects the headless viewport, mirroring the client-side "Download as +# Image/PDF" path (see FORCE_IN_VIEW_EVENT in +# superset-frontend/src/dashboard/constants.ts and forceLoadAllCharts in +# superset-frontend/src/utils/downloadUtils.ts). The non-tiled report capture +# takes a single full-page screenshot that includes below-the-fold holders, so +# those holders must be forced to render before the readiness wait -- otherwise +# a virtualized (or still-loading) off-screen holder is captured blank. A plain +# Event with no `detail.rowIds` means "force every row", matching the frontend's +# single-pass branch. +FORCE_ALL_CHART_HOLDERS_IN_VIEW_EVENT = "superset-force-all-in-view" +FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS = ( + f"() => window.dispatchEvent(new Event('{FORCE_ALL_CHART_HOLDERS_IN_VIEW_EVENT}'))" +) + + +def _unready_chart_holders_js_body(*, viewport_only: bool) -> str: + """Return the shared holder-readiness scan body. + + A holder is ready only after a terminal marker appears and its loading + marker disappears. When ``viewport_only`` is True the scan skips holders + that do not intersect the current viewport: correct for the tiled path, + which scrolls every region into view before capturing it, and for + thumbnails, which only ever capture the viewport. The non-tiled *report* + capture takes a single full-page screenshot that includes below-the-fold + holders, so it must scan every mounted holder (``viewport_only=False``) -- + otherwise an off-screen holder that never rendered is captured blank and + silently delivered as a Success. + """ + viewport_skip = ( + """ const r = holder.getBoundingClientRect(); - if (!(r.top < window.innerHeight && r.bottom > 0)) {{ + if (!(r.top < window.innerHeight && r.bottom > 0)) { continue; - }} + }""" + if viewport_only + else "" + ) + return f""" + const holders = document.querySelectorAll('{CHART_HOLDER_SELECTOR}'); + const unready = []; + for (const holder of holders) {{{viewport_skip} const hasSliceContainer = holder.querySelector( '{SLICE_CONTAINER_SELECTOR}' ) !== null; @@ -206,6 +239,13 @@ class TiledScreenshotBudgetExceededError(ScreenshotTaskBudgetExceededError): }} """ + +# Viewport-scoped scan (tiled path + thumbnails). +UNREADY_CHART_HOLDERS_JS_BODY = _unready_chart_holders_js_body(viewport_only=True) +# Full-dashboard scan (non-tiled report capture, which screenshots the whole +# element in one shot and therefore cannot ignore below-the-fold holders). +UNREADY_ALL_CHART_HOLDERS_JS_BODY = _unready_chart_holders_js_body(viewport_only=False) + # Diagnostic query for every chart holder, including terminal and virtualized # states. It interpolates the same selector constants as the predicates. FIND_CHART_HOLDER_STATES_JS = f""" @@ -256,12 +296,24 @@ class TiledScreenshotBudgetExceededError(ScreenshotTaskBudgetExceededError): f"() => {{ {UNREADY_CHART_HOLDERS_JS_BODY} " "return holders.length > 0 && unready.length === 0; }" ) +# Report readiness for the non-tiled full-page capture: every mounted holder -- +# including below-the-fold ones -- must be terminally rendered. Off-screen +# holders are forced to render first (FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS); if any +# still fails to render within budget the wait times out and the report fails +# loudly rather than shipping a blank/partial screenshot as a Success. +REPORT_ALL_CHART_HOLDERS_READY_JS = ( + f"() => {{ {UNREADY_ALL_CHART_HOLDERS_JS_BODY} " + "return holders.length > 0 && unready.length === 0; }" +) CHART_HOLDERS_MOUNTED_JS = ( f"() => document.querySelectorAll('{CHART_HOLDER_SELECTOR}').length > 0" ) FIND_UNREADY_CHART_HOLDERS_JS = ( f"() => {{ {UNREADY_CHART_HOLDERS_JS_BODY} return unready; }}" ) +FIND_ALL_UNREADY_CHART_HOLDERS_JS = ( + f"() => {{ {UNREADY_ALL_CHART_HOLDERS_JS_BODY} return unready; }}" +) # A chart capture has one target rather than dashboard holders, but needs the # same positive terminal-state guarantee and loading exclusion. diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py index 189449376368..e7242dffc9fe 100644 --- a/superset/utils/webdriver.py +++ b/superset/utils/webdriver.py @@ -34,8 +34,10 @@ CHART_CONTAINER_READY_JS, CHART_CONTAINER_STATE_JS, CHART_HOLDERS_READY_JS, + FIND_ALL_UNREADY_CHART_HOLDERS_JS, FIND_CHART_HOLDER_STATES_JS, - REPORT_CHART_HOLDERS_READY_JS, + FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS, + REPORT_ALL_CHART_HOLDERS_READY_JS, resolve_screenshot_task_budget_seconds, ScreenshotTaskBudgetExceededError, take_tiled_screenshot, @@ -382,7 +384,19 @@ def _wait_for_charts_ready( # noqa: C901 if element_name == "chart-container": readiness_predicate = CHART_CONTAINER_READY_JS elif report_execution_context: - readiness_predicate = REPORT_CHART_HOLDERS_READY_JS + # This non-tiled path captures the whole element in one shot + # (`_get_screenshot` uses `full_page=True` / `element.screenshot()`), + # so below-the-fold holders end up in the image. Force every + # virtualized row to render up front -- mirroring the client-side + # "Download as Image/PDF" path -- and then require *all* mounted + # holders (not just the viewport-visible ones) to reach a terminal + # state. If an off-screen holder never renders, the wait times out + # and the report fails loudly instead of silently delivering a + # blank/partial screenshot as a Success. The tiled path keeps the + # viewport-scoped predicate because it scrolls each region into view + # before capturing it. + page.evaluate(FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS) + readiness_predicate = REPORT_ALL_CHART_HOLDERS_READY_JS else: # Preserve the thumbnail behavior introduced by #42253. The # stricter zero-holder gate is report-specific because an empty @@ -430,6 +444,16 @@ def _wait_for_charts_ready( # noqa: C901 ready_holders = sum( holder.get("state") in ready_states for holder in chart_holder_states ) + # `FIND_CHART_HOLDER_STATES_JS` short-circuits off-screen holders to + # "virtualized" (counted as ready above), so on the report path -- a + # full-page capture that includes below-the-fold holders -- the real + # culprits (off-screen holders that never rendered) would be hidden. + # Surface them explicitly using the non-viewport-scoped scan. + below_fold_unready = ( + page.evaluate(FIND_ALL_UNREADY_CHART_HOLDERS_JS) + if report_execution_context + else unready_chart_holders + ) deadline_elapsed = deadline.elapsed_seconds if deadline else elapsed deadline_remaining = ( deadline.remaining_seconds if deadline else remaining_budget @@ -438,7 +462,8 @@ def _wait_for_charts_ready( # noqa: C901 "report_readiness_terminal url=%s expected_holders=%s " "mounted_holders=%s ready_holders=%s elapsed_seconds=%.2f " "remaining_seconds=%s effective_wait_seconds=%.2f%s " - "terminal_reason=readiness_timeout unready_holders=%s states=%s; " + "terminal_reason=readiness_timeout unready_holders=%s " + "all_unready_holders=%s states=%s; " "aborting before capture or delivery", url, expected_holders, @@ -453,6 +478,7 @@ def _wait_for_charts_ready( # noqa: C901 effective_load_wait, context_suffix, unready_chart_holders, + below_fold_unready, chart_holder_states, ) raise @@ -747,9 +773,21 @@ def get_screenshot( # pylint: disable=too-many-locals, too-many-statements # n context_suffix, ) - # Use tiled screenshots for large dashboards + # Use tiled screenshots for large dashboards. For scheduled + # reports a likely-large dashboard whose measured height is + # at or below a single tile is almost always mid-layout + # (charts still virtualized/collapsed at measurement time), + # not genuinely short -- a 52-chart dashboard is never really + # tile_height + height_unknown + or dashboard_height > tile_height + or report_execution_context is not None ) if use_tiled: diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py index bc245a0ec36e..d2b3885c93d6 100644 --- a/tests/unit_tests/utils/webdriver_test.py +++ b/tests/unit_tests/utils/webdriver_test.py @@ -637,6 +637,91 @@ def evaluate_side_effect(script): "", ) + @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True) + @patch("superset.utils.webdriver._browser_manager") + @patch("superset.utils.webdriver.take_tiled_screenshot") + def test_large_report_dashboard_tiles_even_when_measured_height_is_short( + self, mock_take_tiled, mock_browser_manager + ): + """Regression: the GOOD (full) report path is the tiled one; BAD + (blank/partial) runs mis-route a large dashboard to the single-shot + non-tiled capture because ``scrollHeight`` is measured while charts are + still virtualized/collapsed (<= one tile). A scheduled report whose + dashboard is large by chart count must take the tiled path regardless + of that stale height measurement, so every region is scrolled into + view and waited on instead of captured as a windowed partial. + """ + mock_user = MagicMock() + mock_user.username = "test_user" + + mock_browser = MagicMock() + mock_context = MagicMock() + mock_page = MagicMock() + mock_element = MagicMock() + mock_chart_container = MagicMock() + + mock_browser_manager.get_browser.return_value = mock_browser + mock_browser.new_context.return_value = mock_context + mock_context.new_page.return_value = mock_page + + def locator_side_effect(selector): + if selector == ".chart-container": + locator = MagicMock() + locator.all.return_value = [mock_chart_container] + return locator + return mock_element + + mock_page.locator.side_effect = locator_side_effect + mock_take_tiled.return_value = b"tiled_screenshot" + + def evaluate_side_effect(script): + if script == 'document.querySelectorAll(".chart-container").length': + return 52 # mounted containers + if "const target = document.querySelector" in script: + # Non-zero but <= one tile: the classic mid-layout measurement + # that previously vetoed tiling and dropped to the non-tiled + # path. + return 1500 + return None + + mock_page.evaluate.side_effect = evaluate_side_effect + + with patch("superset.utils.webdriver.app") as mock_app: + mock_app.config = { + "WEBDRIVER_OPTION_ARGS": [], + "WEBDRIVER_WINDOW": {"pixel_density": 1}, + "SCREENSHOT_PLAYWRIGHT_DEFAULT_TIMEOUT": 30000, + "SCREENSHOT_PLAYWRIGHT_WAIT_EVENT": "networkidle", + "SCREENSHOT_SELENIUM_HEADSTART": 1, + "SCREENSHOT_SELENIUM_ANIMATION_WAIT": 1, + "SCREENSHOT_LOCATE_WAIT": 10, + "SCREENSHOT_LOAD_WAIT": 10, + "SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False, + "SCREENSHOT_TILED_ENABLED": True, + "SCREENSHOT_TILED_CHART_THRESHOLD": 20, + "SCREENSHOT_TILED_HEIGHT_THRESHOLD": 5000, + # Larger than the measured 1500px height, so only the new + # report-mode branch (not `dashboard_height > tile_height`) can + # select tiling here. + "SCREENSHOT_TILED_VIEWPORT_HEIGHT": 2000, + } + + with patch.object(WebDriverPlaywright, "auth") as mock_auth: + mock_auth.return_value = mock_context + + driver = WebDriverPlaywright("chrome") + result = driver.get_screenshot( + "http://example.com/dashboard/805", + "standalone", + mock_user, + report_execution_context=_report_context(), + ) + + assert result == b"tiled_screenshot" + mock_take_tiled.assert_called_once() + # The non-tiled single-shot capture must not run for this large report. + mock_page.screenshot.assert_not_called() + @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True) @patch("superset.utils.webdriver._browser_manager") @patch("superset.utils.webdriver.logger") @@ -1168,6 +1253,103 @@ def test_report_readiness_budget_exhaustion_skips_poll_and_capture(self): page.wait_for_function.assert_not_called() page.screenshot.assert_not_called() + def test_report_readiness_forces_below_fold_render_and_waits_for_all_holders( + self, + ): + """Regression for blank/partial report PDFs. + + The non-tiled report capture takes a single full-page screenshot that + includes below-the-fold holders, so the readiness gate must (a) force + every virtualized row to render up front and (b) require *all* mounted + holders -- not just the viewport-visible ones -- to reach a terminal + state. Otherwise an off-screen holder that never rendered is captured + blank and silently delivered as a Success. + """ + from superset.utils.screenshot_utils import ( + FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS, + REPORT_ALL_CHART_HOLDERS_READY_JS, + ) + + page = MagicMock() + page.evaluate.return_value = [{"chartId": "7", "state": "rendered"}] + + WebDriverPlaywright._wait_for_charts_ready( + page, + "http://example.com/dashboard/805", + 5, + "standalone", + report_execution_context=_report_context(), + ) + + # (a) Off-screen rows are forced to render before the wait. + assert any( + call.args and call.args[0] == FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS + for call in page.evaluate.call_args_list + ) + # (b) The readiness predicate is the all-holders variant: it must not + # skip below-the-fold holders (no viewport-intersection test), so a + # virtualized/unrendered off-screen holder cannot satisfy the gate. + predicate = page.wait_for_function.call_args.args[0] + assert predicate == REPORT_ALL_CHART_HOLDERS_READY_JS + assert "getBoundingClientRect" not in predicate + assert "window.innerHeight" not in predicate + + @patch("superset.utils.webdriver.logger") + def test_report_readiness_below_fold_unrendered_fails_loudly(self, mock_logger): + """When an off-screen holder never renders within budget the report + must fail loudly (raise) rather than capture/deliver a blank + screenshot, and the terminal log must surface the below-the-fold + unready holders that the viewport-scoped diagnostic hides as + 'virtualized'. + """ + from superset.utils.screenshot_utils import ( + FIND_ALL_UNREADY_CHART_HOLDERS_JS, + FIND_CHART_HOLDER_STATES_JS, + ) + from superset.utils.webdriver import PlaywrightTimeout + + page = MagicMock() + # 22 on-screen rendered holders + 30 off-screen holders that the + # viewport-scoped diagnostic labels 'virtualized' (and would otherwise + # count as "ready"). + holder_states = [ + {"chartId": str(i), "state": "rendered"} for i in range(22) + ] + [{"chartId": str(i), "state": "virtualized"} for i in range(22, 52)] + below_fold_unready = [ + {"chartId": str(i), "state": "nothing_mounted"} for i in range(22, 52) + ] + + def _evaluate(script, *args): + if script == FIND_ALL_UNREADY_CHART_HOLDERS_JS: + return below_fold_unready + if script == FIND_CHART_HOLDER_STATES_JS: + return holder_states + return None + + page.evaluate.side_effect = _evaluate + page.wait_for_function.side_effect = PlaywrightTimeout( + "below-fold holders never rendered" + ) + + with pytest.raises(PlaywrightTimeout): + WebDriverPlaywright._wait_for_charts_ready( + page, + "http://example.com/dashboard/805", + 5, + "standalone", + report_execution_context=_report_context(), + ) + + terminal_call = next( + call + for call in mock_logger.warning.call_args_list + if call.args and call.args[0].startswith("report_readiness_terminal") + ) + assert "terminal_reason=readiness_timeout" in terminal_call.args[0] + # The below-the-fold offenders are surfaced explicitly. + assert "all_unready_holders=" in terminal_call.args[0] + assert below_fold_unready in terminal_call.args + @patch("superset.utils.webdriver.logger") def test_chart_capture_ready_logs_container_state_not_holder_counts( self, mock_logger