Notebooks: Remote SVG Support (#11046)

dhruvisompura · web-flow · commit 440aa6169b36 · 2025-12-12T16:00:57.000-08:00
Addresses #10594 This PR addresses #10594 which adds remote SVG rendering support in Positron Notebooks to bring in parity with built-in notebooks. Remote image rendering already works for other image formats, this change specifically handled SVGs. ## Explanation Built-in notebooks render markdown cells inside a webview and the markdown rendering is done via an extension on the extension host thread. When images (including remote SVGs) are loaded from within this webview, the request comes from a webview frame. In `src/vs/code/electron-main/app.ts`, there is logic that blocks remote SVGs by default, but allows them if the request comes from a "safe context" (i.e. a webview frame). This is how built-in notebooks are able to render remote SVGs. Positron notebooks render markdown in the main thread which means that requests for remote SVGs come from the main window frame, which is not considered a "safe context". When a remote SVG is loaded from Positron notebooks: 1. The request comes from the main workbench frame 2. The `isSafeFrame()` function (`electron-main/app.ts:225`) returns false (no webview in the frame hierarchy) 3. The SVG request is blocked by the security check in `electron-main/app.ts:301` ## Solution The solution in this PR is to fetch remote SVGs and then converts them to base64 data URLs via the positron-notebooks extension. The base64 data URL is then sent back to the main thread and rendered in the notebook. NOTE: Because it can take time to fetch remote images, we show a placeholder where the image should be while we fetch the image. This is more obvious with svgs than other remote image formats and is visible in the video below. When reloading the Positron window or on startup this is pretty noticeable as of right now. ## Screenshots **render svg - new md cell** https://github.com/user-attachments/assets/32ab381a-864f-4bec-9d50-1dab1da30902 **render svg - window reload** https://github.com/user-attachments/assets/1766fa82-c43c-4b61-9481-c15a477d37a5 **render svg - view to edit mode transition** https://github.com/user-attachments/assets/4457bbcf-16c2-402b-a0e9-3cda845abfb5 ### Release Notes  #### New Features - N/A #### Bug Fixes - N/A ### QA Notes @:positron-notebooks I used the following markdown for testing this change: ``` ### these are the remote images being used Reference url with .svg ![Alt text](https://www.dataquest.io/wp-content/uploads/2023/02/DQ-Logo.svg) Reference positron image url ![test image](https://positron.posit.co/assets/images/positron-logo_fullcolor-TM.png) ```
diff --git a/extensions/positron-notebooks/package.json b/extensions/positron-notebooks/package.json
@@ -19,6 +19,10 @@
       {
         "command": "positronNotebookHelpers.convertImageToBase64",
         "title": "Convert Image to Base64"
+      },
+      {
+        "command": "positronNotebookHelpers.fetchRemoteImage",
+        "title": "Fetch Remote Image"
       }
     ]
   },
diff --git a/extensions/positron-notebooks/src/extension.ts b/extensions/positron-notebooks/src/extension.ts
@@ -6,10 +6,12 @@
 import path from 'path';
 import * as vscode from 'vscode';
 import { readFile } from 'fs';
+import * as https from 'https';
+import * as http from 'http';
 
 // Make sure this matches the error message type defined where used
 // (src/vs/workbench/contrib/positronNotebook/browser/notebookCells/DeferredImage.tsx)
-type CoversionErrorMsg = {
+type ConversionErrorMsg = {
 	status: 'error';
 	message: string;
 };
@@ -23,7 +25,7 @@ export function activate(context: vscode.ExtensionContext) {
 	context.subscriptions.push(
 		vscode.commands.registerCommand(
 			'positronNotebookHelpers.convertImageToBase64',
-			async (imageSrc: string, baseLoc: string) => new Promise<string | CoversionErrorMsg>((resolve) => {
+			async (imageSrc: string, baseLoc: string) => new Promise<string | ConversionErrorMsg>((resolve) => {
 				const fullImagePath = path.join(baseLoc, imageSrc);
 				const fileExtension = path.extname(imageSrc).slice(1);
 				const mimeType = mimeTypeMap[fileExtension.toLowerCase()];
@@ -52,13 +54,95 @@ export function activate(context: vscode.ExtensionContext) {
 					});
 				} catch (e) {
 					return {
-						type: 'error',
+						status: 'error',
 						message: e instanceof Error ? e.message : `Error occured while converting image ${fullImagePath} to base64.`,
 					};
 				}
 			})
 		)
 	);
+
+	/**
+	 * Command that fetches a remote image and converts it to a base64 data URL.
+	 *
+	 * This command is used to load remote SVG images in Positron notebooks.
+	 * Positron notebooks render markdown directly in the main window context
+	 * and not in a webview like built-in VS Code notebooks. VS Code blocks
+	 * remote SVGs unless it is for the notebook webview (see `src/vs/code/electron-main/app.ts`)
+	 */
+	context.subscriptions.push(
+		vscode.commands.registerCommand(
+			'positronNotebookHelpers.fetchRemoteImage',
+			(imageUrl: string) => new Promise<string | ConversionErrorMsg>((resolve) => {
+				// Determine protocol
+				const protocol = imageUrl.startsWith('https:') ? https : http;
+
+				try {
+					protocol.get(imageUrl, (response) => {
+						// Check for successful response
+						if (response.statusCode !== 200) {
+							resolve({
+								status: 'error',
+								message: `Failed to fetch image: HTTP ${response.statusCode}`,
+							});
+							return;
+						}
+
+						// Get MIME type from response headers or infer from URL
+						let mimeType = response.headers['content-type'];
+						if (!mimeType || !mimeType.startsWith('image/')) {
+							// Try to infer from URL extension
+							const urlExtension = path.extname(new URL(imageUrl).pathname).slice(1);
+							mimeType = mimeTypeMap[urlExtension.toLowerCase()];
+							if (!mimeType) {
+								resolve({
+									status: 'error',
+									message: `Unsupported or missing content type: ${response.headers['content-type']}`,
+								});
+								return;
+							}
+						}
+
+						// Collect response data
+						const chunks: Buffer[] = [];
+						response.on('data', (chunk) => {
+							chunks.push(Buffer.from(chunk));
+						});
+
+						response.on('end', () => {
+							try {
+								const buffer = Buffer.concat(chunks);
+								const base64 = buffer.toString('base64');
+								resolve(`data:${mimeType};base64,${base64}`);
+							} catch (e) {
+								resolve({
+									status: 'error',
+									message: e instanceof Error ? e.message : 'Failed to convert image to base64',
+								});
+							}
+						});
+
+						response.on('error', (err) => {
+							resolve({
+								status: 'error',
+								message: err.message,
+							});
+						});
+					}).on('error', (err) => {
+						resolve({
+							status: 'error',
+							message: err.message,
+						});
+					});
+				} catch (e) {
+					resolve({
+						status: 'error',
+						message: e instanceof Error ? e.message : `Error occurred while fetching image ${imageUrl}`,
+					});
+				}
+			})
+		)
+	);
 }
 
 
diff --git a/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/DeferredImage.tsx b/src/vs/workbench/contrib/positronNotebook/browser/notebookCells/DeferredImage.tsx
@@ -22,17 +22,17 @@ import { usePositronReactServicesContext } from '../../../../../base/browser/pos
  * This should match the error message defined in the command definition
  * (extensions/positron-notebooks/src/extension.ts)
  */
-type CoversionErrorMsg = {
+type ConversionErrorMsg = {
 	status: 'error';
 	message: string;
 };
 
 /**
  * Predicate function to allow us to be safe with our response processing from command.
- * @param x: Variable of unknown type to check if it is a `CoversionErrorMsg`.
- * @returns Whether the object is a `CoversionErrorMsg`.
+ * @param x: Variable of unknown type to check if it is a `ConversionErrorMsg`.
+ * @returns Whether the object is a `ConversionErrorMsg`.
  */
-function isConversionErrorMsg(x: unknown): x is CoversionErrorMsg {
+function isConversionErrorMsg(x: unknown): x is ConversionErrorMsg {
 	return x !== null && typeof x === 'object' && 'status' in x && x.status === 'error' && 'message' in x;
 }
 
@@ -46,64 +46,144 @@ type ImageDataResults = {
 	message: string;
 };
 
+const REMOTE_SVG_TIMEOUT_MS = 5000;
+const CONVERSION_TIMEOUT_MS = 3000;
+const ERROR_TIMEOUT_MS = 1000;
+
 /**
  * Special image component that defers loading of the image while it converts it to a data-url using
- * the `positronNotebookHelpers.convertImageToBase64` command.
+ * using a command from the `positronNotebookHelpers` extension.
  * @param props: Props for `img` element.
  * @returns Image tag that shows the image once it is loaded.
  */
-// eslint-disable-next-line react/prop-types
 export function DeferredImage({ src = 'no-source', ...props }: React.ComponentPropsWithoutRef<'img'>) {
 	const services = usePositronReactServicesContext();
 	const notebookInstance = useNotebookInstance();
-	const baseLocation = getNotebookBaseUri(notebookInstance.uri).path;
 
 	const [results, setResults] = React.useState<ImageDataResults>({ status: 'pending' });
 
 	React.useEffect(() => {
+		/**
+		 * Shared helper to handle image conversion.
+		 *
+		 * @param commandName The command to execute
+		 * @param commandArgs Arguments to pass to the command
+		 * @param timeoutMs Timeout in milliseconds for the operation
+		 * @returns A cleanup function to cancel ongoing operations
+		 */
+		const handleImageConversion = (
+			commandName: string,
+			commandArgs: unknown[],
+			timeoutMs: number
+		): (() => void) => {
+			let delayedErrorMsg: Timeout;
+
+			// Create cancelable promise to execute the command with timeout
+			const conversionCancellablePromise = createCancelablePromise(() => raceTimeout(
+				services.commandService.executeCommand(commandName, ...commandArgs),
+				timeoutMs
+			));
+
+			// Handle the conversion result
+			conversionCancellablePromise.then((payload) => {
+				if (typeof payload === 'string') {
+					// Success: got base64 data URL
+					setResults({ status: 'success', data: payload });
+				} else if (isConversionErrorMsg(payload)) {
+					// Known error from the command
+					delayedErrorMsg = setTimeout(() => {
+						services.logService.error(
+							localize('notebook.remoteImage.failedToConvert', "{0} - Failed to convert:", commandName),
+							commandArgs[0], // image source
+							payload.message
+						);
+					}, ERROR_TIMEOUT_MS);
+					setResults(payload);
+				} else {
+					// Unexpected response format
+					const unexpectedResponseString = localize('unexpectedResponse', "Unexpected response from {0}", commandName);
+					delayedErrorMsg = setTimeout(() => {
+						services.logService.error(unexpectedResponseString, payload);
+					}, ERROR_TIMEOUT_MS);
+					setResults({ status: 'error', message: unexpectedResponseString });
+				}
+			}).catch((err) => {
+				// Promise was rejected (timeout or other error)
+				setResults({ status: 'error', message: err.message });
+			});
+
+			// Return cleanup function for React effect
+			return () => {
+				clearTimeout(delayedErrorMsg);
+				conversionCancellablePromise.cancel();
+			};
+		};
 
-		// Check for prefix of http or https to avoid converting remote images
-		if (src.startsWith('http://') || src.startsWith('https://')) {
-			setResults({ status: 'success', data: src });
-			return;
-		}
-
-		const conversionTimeoutMs = 3000;
-		const errorTimeoutMs = 1000;
-
-		let delayedErrorMsg: Timeout;
-
-		const conversionCancellablePromise = createCancelablePromise(() => raceTimeout(
-			services.commandService.executeCommand('positronNotebookHelpers.convertImageToBase64', src, baseLocation),
-			conversionTimeoutMs
-		));
+		/**
+		 * Handles fetching and converting remote SVG images to base64 data URLs.
+		 *
+		 * @param imageUrl The SVG URL to fetch
+		 * @returns Cleanup function to cancel ongoing operations
+		 */
+		const handleRemoteSvg = (imageUrl: string): (() => void) => {
+			return handleImageConversion(
+				'positronNotebookHelpers.fetchRemoteImage',
+				[imageUrl],
+				REMOTE_SVG_TIMEOUT_MS
+			);
+		};
 
-		conversionCancellablePromise.then((payload) => {
-			if (typeof payload === 'string') {
-				setResults({ status: 'success', data: payload });
-			} else if (isConversionErrorMsg(payload)) {
+		/**
+		 * Handles converting local/relative image paths to base64 data URLs.
+		 *
+		 * @param imagePath The relative image path
+		 * @param baseLocation The base directory to resolve relative paths from
+		 * @returns Cleanup function to cancel ongoing operations
+		 */
+		const handleLocalImage = (imagePath: string, baseLocation: string): (() => void) => {
+			return handleImageConversion(
+				'positronNotebookHelpers.convertImageToBase64',
+				[imagePath, baseLocation],
+				CONVERSION_TIMEOUT_MS
+			);
+		};
 
-				delayedErrorMsg = setTimeout(() => {
-					services.logService.error(localize('failedToConvert', 'Failed to convert image to base64:'), src, payload.message);
-				}, errorTimeoutMs);
+		/* ---- Main useEffect logic starts here ---- */
 
-				setResults(payload);
+		// Check for remote images (http/https URLs)
+		if (src.startsWith('http://') || src.startsWith('https://')) {
+			/**
+			 * Remote SVGs are blocked by VS Code's security policy when loaded directly
+			 * in the main window context (see `src/vs/code/electron-main/app.ts:221-303`).
+			 * We safely handle SVGs by fetching the svg via the extension host and
+			 * converting to base64 data URLs.
+			 *
+			 * Other formats (PNG, JPG, etc.) work natively and can be loaded directly.
+			 */
+			const isSvg = isSvgUrl(src);
+			if (isSvg) {
+				// Handle remote SVG through extension
+				return handleRemoteSvg(src);
 			} else {
-				const unexpectedResponseString = localize('unexpectedResponse', 'Unexpected response from convertImageToBase64');
-				delayedErrorMsg = setTimeout(() => {
-					services.logService.error(unexpectedResponseString, payload);
-				}, errorTimeoutMs);
-				setResults({ status: 'error', message: unexpectedResponseString });
+				// Non-SVG remote images (PNG, JPG, etc.) work natively, use direct URL
+				setResults({ status: 'success', data: src });
+				return;
 			}
-		}).catch((err) => {
-			setResults({ status: 'error', message: err.message });
-		});
+		}
 
-		return () => {
-			clearTimeout(delayedErrorMsg);
-			conversionCancellablePromise.cancel();
-		};
-	}, [src, baseLocation, services]);
+		// Otherwise, handle local/relative image paths
+		let baseLocation: string;
+		try {
+			// Get base location to resolve relative paths
+			baseLocation = getNotebookBaseUri(notebookInstance.uri).path;
+		} catch (error) {
+			setResults({ status: 'error', message: String(error) });
+			return;
+		}
+
+		// Convert local image to base64 data URL
+		return handleLocalImage(src, baseLocation);
+	}, [src, notebookInstance, services]);
 
 	switch (results.status) {
 		case 'pending':
@@ -122,6 +202,25 @@ export function DeferredImage({ src = 'no-source', ...props }: React.ComponentPr
 	}
 }
 
+/**
+ * Detects if a URL points to an SVG file based on its path extension.
+ *
+ * @param url The URL to check
+ * @returns true if the URL path ends with .svg extension
+ */
+function isSvgUrl(url: string): boolean {
+	try {
+		const urlObj = new URL(url);
+		// Get the pathname without query params or fragments
+		const pathname = urlObj.pathname.toLowerCase();
+		// Check if it ends with .svg
+		return pathname.endsWith('.svg');
+	} catch {
+		// If URL parsing fails, fall back to simple check
+		return url.toLowerCase().endsWith('.svg');
+	}
+}
+
 function getNotebookBaseUri(notebookUri: URI) {
 	if (notebookUri.scheme === Schemas.untitled) {
 		// TODO: Use workspace context service to set the base URI to workspace root

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,10 @@`
`19`	`19`	`{`
`20`	`20`	`"command": "positronNotebookHelpers.convertImageToBase64",`
`21`	`21`	`"title": "Convert Image to Base64"`
	`22`	`+ },`
	`23`	`+ {`
	`24`	`+ "command": "positronNotebookHelpers.fetchRemoteImage",`
	`25`	`+ "title": "Fetch Remote Image"`
`22`	`26`	`}`
`23`	`27`	`]`
`24`	`28`	`},`