sadpandajoe commented on code in PR #43340:
URL: https://github.com/apache/superset/pull/43340#discussion_r3866913392


##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -315,35 +337,37 @@ export const useDownloadMenuItems = (
     ...(userCanExport
       ? [
           {
-            key: "export-xlsx",
-            label: t("Export Data to Excel"),
-            onClick: () => onExportXlsx("data"),
+            key: 'export-xlsx',
+            label: t('Export Data to Excel'),
+            onClick: () => onExportXlsx('data'),
           },
           // Image export renders charts through the headless webdriver, so 
only
           // offer it where that infrastructure is available (same signal as 
the
           // PDF/PNG image downloads above); otherwise non-table charts would
-          // silently come back empty.
-          ...(isWebDriverScreenshotEnabled
+          // silently come back empty. Embedded sessions are excluded too: the
+          // webdriver cannot render Explore under a guest identity, so the
+          // export would burn its whole task budget and produce nothing.
+          ...(isWebDriverScreenshotEnabled && !isEmbedded

Review Comment:
   Hiding this item only in the menu does not prevent a guest-token client from 
posting `mode=images` directly. The API then accepts an export the change 
itself says cannot render under a guest identity, holding the shared guest 
dashboard lock while the task runs to timeout. Could the API reject image mode 
for guest requests as well?



##########
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/index.tsx:
##########
@@ -83,9 +89,28 @@ export const useDownloadMenuItems = (
     canExportImage,
   } = props;
 
-  const { addDangerToast, addSuccessToast } = useToasts();
+  const { addDangerToast, addSuccessToast, addInfoToast } = useToasts();
   const dataMask = useSelector((state: RootState) => state.dataMask);
-  const SCREENSHOT_NODE_SELECTOR = ".dashboard";
+  // Embedded (iframe) sessions may have no email address, so they get
+  // delivery-neutral copy and a poll window that outlives the task budget.
+  const isEmbedded = isEmbeddedDashboard();

Review Comment:
   `isEmbedded()` is frame detection rather than a guest-principal check. An 
authenticated user running Superset in an iframe now loses image export and 
receives the no-email copy even though their identity can render images and 
receive email. Could this branch use the session principal instead of iframe 
presence?



-- 
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]

Reply via email to