Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix copy/paste and context menus for Shiny and other content in Viewer pane #3430

Merged
merged 25 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c4b1464
expose a way to set URIs directly on a webview (WIP)
jmcphers May 30, 2024
5257228
initial webview uri plumbing
jmcphers May 30, 2024
1a98d77
add a hook to execute js in the webview
jmcphers May 30, 2024
1c1b1bd
provide a way to await frame creation; begin migrating event handlers
jmcphers May 31, 2024
d0d81a1
return a real frame from event
jmcphers May 31, 2024
7968cb7
we can only pass plain objects across electron boundary
jmcphers May 31, 2024
40eba9e
wait for target url before resolving frame
jmcphers May 31, 2024
832482d
complete forwarding of context menu events
jmcphers May 31, 2024
dddb8e9
first working example
jmcphers May 31, 2024
f0fae06
better detection of load finish events
jmcphers May 31, 2024
be14eaf
clean up comments and logging
jmcphers May 31, 2024
6e3b53d
document webview-events.js
jmcphers May 31, 2024
6f86d53
begin working on reload/nav
jmcphers May 31, 2024
f70fdde
remove inner click handler; finish connecting nav messages
jmcphers Jun 1, 2024
6a1d2e4
Merge remote-tracking branch 'origin/main' into bugfix/copy-paste-viewer
jmcphers Jun 4, 2024
d245ee9
track navigation and re-inject script
jmcphers Jun 4, 2024
134ff13
Merge remote-tracking branch 'origin/main' into bugfix/copy-paste-viewer
jmcphers Jun 5, 2024
0b6be6c
hide nonce in user-facing preview bar
jmcphers Jun 5, 2024
a0ee1c8
less aggressive reload (to preserve history)
jmcphers Jun 5, 2024
12aa6de
ask Positron to open external URLs
jmcphers Jun 6, 2024
e0a8c77
clean up unused event; add docs
jmcphers Jun 6, 2024
d8ab5f8
use electron dependency injection to toggle old/new behavior
jmcphers Jun 6, 2024
aca57ff
Merge remote-tracking branch 'origin/main' into bugfix/copy-paste-viewer
jmcphers Jun 6, 2024
607861d
Merge remote-tracking branch 'origin/main' into bugfix/copy-paste-viewer
jmcphers Jun 10, 2024
bc8da7d
simplify access to pending navigations
jmcphers Jun 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/vs/code/electron-main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,13 @@ export class CodeApplication extends Disposable {
};

const isAllowedWebviewRequest = (uri: URI, details: Electron.OnBeforeRequestListenerDetails): boolean => {
if (uri.path !== '/index.html') {
// --- Start Positron ---
// Add index-external.html to the allowlist. Positron uses this file
// to load external URLs into weviews.
if (uri.path !== '/index.html' && uri.path !== '/index-external.html') {
return true; // Only restrict top level page of webviews: index.html
}
// --- End Positron ---

const frame = details.frame;
if (!frame || !this.windowsMainService) {
Expand Down
52 changes: 52 additions & 0 deletions src/vs/platform/webview/common/webviewManagerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,28 @@ export interface WebviewWindowId {
}

// --- Start Positron ---
/**
* A unique composite identifier for a frame inside a webview.
*/
export interface WebviewFrameId {
/** The process ID that backs the frame */
readonly processId: number;

/** The frame's routing identifier */
readonly routingId: number;
}

/**
* An event fired when a frame navigates to a new URL.
*/
export interface FrameNavigationEvent {
/** The ID of the frame that navigated */
readonly frameId: WebviewFrameId;

/** The frame's new URL */
readonly url: string;
}

export interface WebviewRectangle {
readonly x: number;
readonly y: number;
Expand Down Expand Up @@ -55,6 +77,36 @@ export interface IWebviewManagerService {
stopFindInFrame(windowId: WebviewWindowId, frameName: string, options: { keepSelection?: boolean }): Promise<void>;

// --- Start Positron ---
/**
* Waits for a frame with the given target URL to be created in a webview;
* when it has been created, returns the frame id.
*
* @param windowId The window id of the webview in which the frame is to be created.
* @param targetUrl The URL of the frame to wait for.
*/
awaitFrameCreation(windowId: WebviewWindowId, targetUrl: string): Promise<WebviewFrameId>;

/**
* An event fired when a webview frame has navigated to a new URL.
*/
onFrameNavigation: Event<FrameNavigationEvent>;

/**
* Capture a snapshot of the contents of a webview as a PNG image.
*
* @param windowId The window id of the webview to capture.
* @param area The area of the webview to capture, in CSS pixels.
*/
captureContentsAsPng(windowId: WebviewWindowId, area?: WebviewRectangle): Promise<VSBuffer | undefined>;

/**
* Execute JavaScript code in a webview frame.
*
* @param frameId The ID of the frame in which to execute the code.
* @param code The code to execute.
*
* @returns A promise that resolves to the result of the code execution.
*/
executeJavaScript(frameId: WebviewFrameId, code: string): Promise<any>;
// --- End Positron ---
}
103 changes: 102 additions & 1 deletion src/vs/platform/webview/electron-main/webviewMainService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ import { IWindowsMainService } from 'vs/platform/windows/electron-main/windows';

// --- Start Positron ---
// eslint-disable-next-line no-duplicate-imports
import { Rectangle } from 'electron';
import { Rectangle, webFrameMain } from 'electron';
import { VSBuffer } from 'vs/base/common/buffer';

// eslint-disable-next-line no-duplicate-imports
import { IDisposable } from 'vs/base/common/lifecycle';

// eslint-disable-next-line no-duplicate-imports
import { WebviewFrameId, FrameNavigationEvent } from 'vs/platform/webview/common/webviewManagerService';
import { DeferredPromise } from 'vs/base/common/async';
// --- End Positron ---

export class WebviewMainService extends Disposable implements IWebviewManagerService {
Expand Down Expand Up @@ -91,6 +98,21 @@ export class WebviewMainService extends Disposable implements IWebviewManagerSer
}

// --- Start Positron ---

// The onFrameNavigated event is fired when a frame in a webview navigates to
// a new URL.
private readonly _onFrameNavigated = this._register(new Emitter<FrameNavigationEvent>());
public onFrameNavigation = this._onFrameNavigated.event;

// A map of window IDs to disposables for navigation event listeners. We
// attach a single listener to each window to capture frame navigation
// events.
private readonly _navigationListeners = new Map<WebviewWindowId, IDisposable>();

// A map of pending frame navigations, from the URL of the frame to the
// promise that will be resolved when a frame navigates to that URL.
private readonly _pendingNavigations = new Map<string, DeferredPromise<WebviewFrameId>>();

/**
* Captures the contents of the webview in the given window as a PNG image.
*
Expand All @@ -110,6 +132,85 @@ export class WebviewMainService extends Disposable implements IWebviewManagerSer
const image = await contents.capturePage(area);
return VSBuffer.wrap(image.toPNG());
}

/**
* Waits for a frame to be created in a webview.
*
* @param windowId The ID of the window containing the webview
* @param targetUrl The URL of the frame to await creation of
* @returns A unique identifier for the frame
*/
public async awaitFrameCreation(windowId: WebviewWindowId, targetUrl: string): Promise<WebviewFrameId> {
// Get the window containing the webview
const window = this.windowsMainService.getWindowById(windowId.windowId);
if (!window?.win) {
throw new Error(`Invalid windowId: ${windowId}`);
}

// If we aren't already listening for navigation events on this window,
// set up a listener to capture them
if (!this._navigationListeners.has(windowId)) {
// Event handler for navigation events
const onNavigated = (_event: any,
url: string,
_httpResponseCode: number,
_httpStatusText: string,
_isMainFrame: boolean,
frameProcessId: number,
frameRoutingId: number) => {
const frameId = { processId: frameProcessId, routingId: frameRoutingId };
this.onFrameNavigated(frameId, url);
};
window.win!.webContents.on('did-frame-navigate', onNavigated);

// Disposable for the listener
const disposable = { dispose: () => window.win!.webContents.off('did-frame-navigate', onNavigated) };
this._navigationListeners.set(windowId, disposable);

// Register the disposable so we can clean up when the service is
// disposed
this._register(disposable);
}

// Create a new deferred promise; it will be resolved when the frame
// navigates to the target URL.
const deferred = new DeferredPromise<WebviewFrameId>();
this._pendingNavigations.set(targetUrl, deferred);
return deferred.p;
}

/**
* Executes a JavaScript code snippet in a webview frame.
*
* @param frameId The ID of the frame in which to execute the code.
* @param script The code to execute, as a string.
* @returns The result of evaluating the code.
*/
public async executeJavaScript(frameId: WebviewFrameId, script: string): Promise<any> {
const frame = webFrameMain.fromId(frameId.processId, frameId.routingId);
if (!frame) {
throw new Error(`No frame found with frameId: ${JSON.stringify(frameId)}`);
}
return frame.executeJavaScript(script);
}

/**
* Handles a frame navigation event.
* @param frameId The ID of the frame that navigated
* @param url The URL to which the frame navigated
*/
private onFrameNavigated(frameId: WebviewFrameId, url: string): void {
this._onFrameNavigated.fire({ frameId, url });

// Check for any pending navigations that match this URL; if we find
// any, complete them
if (this._pendingNavigations.has(url)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny idea: You can just call get and avoid has then get.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Done in bc8da7d

const deferred = this._pendingNavigations.get(url);
deferred!.complete(frameId);
this._pendingNavigations.delete(url);
}
}

// --- End Positron ---

private getFrameByName(windowId: WebviewWindowId, frameName: string): WebFrameMain {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ export class WebviewProtocolProvider extends Disposable {
private static validWebviewFilePaths = new Map([
['/index.html', 'index.html'],
['/fake.html', 'fake.html'],
// --- Start Positron ---
// Add 'index-external' to the list of valid webview file paths. This is
// used as a host iframe when loading external URLs in Positron.
['/index-external.html', 'index-external.html'],
// --- End Positron ---
['/service-worker.js', 'service-worker.js'],
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import 'vs/css!./actionBars';
import * as React from 'react';
import { PropsWithChildren, } from 'react'; // eslint-disable-line no-duplicate-imports
import { PropsWithChildren, useEffect, } from 'react'; // eslint-disable-line no-duplicate-imports
import { ICommandService } from 'vs/platform/commands/common/commands';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
Expand All @@ -17,12 +17,13 @@ import { PositronSessionsServices } from 'vs/workbench/contrib/positronRuntimeSe
import { ActionBarRegion } from 'vs/platform/positronActionBar/browser/components/actionBarRegion';
import { ActionBarButton } from 'vs/platform/positronActionBar/browser/components/actionBarButton';
import { localize } from 'vs/nls';
import { PreviewUrl } from 'vs/workbench/contrib/positronPreview/browser/previewUrl';
import { PreviewUrl, QUERY_NONCE_PARAMETER } from 'vs/workbench/contrib/positronPreview/browser/previewUrl';
import { IPositronPreviewService } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewSevice';
import { ActionBarSeparator } from 'vs/platform/positronActionBar/browser/components/actionBarSeparator';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { URI } from 'vs/base/common/uri';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { DisposableStore } from 'vs/base/common/lifecycle';

// Constants.
const kPaddingLeft = 8;
Expand Down Expand Up @@ -69,17 +70,26 @@ export const ActionBars = (props: PropsWithChildren<ActionBarsProps>) => {

// Handler for the navigate back button.
const navigateBackHandler = () => {
props.preview.webview.postMessage({ command: 'back' });
props.preview.webview.postMessage({
channel: 'execCommand',
data: 'navigate-back'
});
};

// Handler for the navigate forward button.
const navigateForwardHandler = () => {
props.preview.webview.postMessage({ command: 'forward' });
props.preview.webview.postMessage({
channel: 'execCommand',
data: 'navigate-forward'
});
};

// Handler for the reload button.
const reloadHandler = () => {
props.preview.webview.postMessage({ command: 'reload' });
props.preview.webview.postMessage({
channel: 'execCommand',
data: 'reload-window'
});
};

// Handler for the clear button.
Expand Down Expand Up @@ -131,6 +141,35 @@ export const ActionBars = (props: PropsWithChildren<ActionBarsProps>) => {
}
};

// useEffect hook.
useEffect(() => {
const disposables = new DisposableStore();
disposables.add(props.preview.onDidNavigate(e => {
if (urlInputRef.current) {
// Remove the nonce from the URL before updating the input; we
// use this this for cache busting but the user doesn't need to
// see it.
if (e.query) {
const nonceIndex = e.query.indexOf(`${QUERY_NONCE_PARAMETER}=`);
if (nonceIndex !== -1) {
const nonceEnd = e.query.indexOf('&', nonceIndex);
if (nonceEnd !== -1) {
e = e.with({
query: e.query.slice(0, nonceIndex) + e.query.slice(nonceEnd + 1)
});
} else {
e = e.with({
query: e.query.slice(0, nonceIndex)
});
}
}
}
urlInputRef.current.value = e.toString();
}
}));
return () => disposables.dispose();
}, [props.preview]);

// Render.
return (
<PositronActionBarContextProvider {...props}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,13 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ViewPaneContainer } from 'vs/workbench/browser/parts/views/viewPaneContainer';
import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions';
import { PositronPreviewViewPane } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewView';
import { PositronPreviewService } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewServiceImpl';
import { IPositronPreviewService, POSITRON_PREVIEW_VIEW_ID } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewSevice';
import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions, IWorkbenchContribution } from 'vs/workbench/common/contributions';
import { ViewContainer, IViewContainersRegistry, ViewContainerLocation, Extensions as ViewContainerExtensions, IViewsRegistry } from 'vs/workbench/common/views';
import { registerAction2 } from 'vs/platform/actions/common/actions';
import { PositronOpenUrlInViewerAction } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewActions';

// Register the Positron preview service.
registerSingleton(IPositronPreviewService, PositronPreviewService, InstantiationType.Delayed);

// The Positron preview view icon.
const positronPreviewViewIcon = registerIcon('positron-preview-view-icon', Codicon.positronPreviewView, nls.localize('positronPreviewViewIcon', 'View icon of the Positron preview view.'));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
*--------------------------------------------------------------------------------------------*/

import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions';
import { PositronPreviewService } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewServiceImpl';
import { IPositronPreviewService } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewSevice';

// Register the Positron preview service.
registerSingleton(IPositronPreviewService, PositronPreviewService, InstantiationType.Delayed);
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { Disposable } from 'vs/base/common/lifecycle';
import { IPositronPreviewService } from 'vs/workbench/contrib/positronPreview/browser/positronPreview';
import { Event, Emitter } from 'vs/base/common/event';
import { IWebviewService, WebviewExtensionDescription, WebviewInitInfo } from 'vs/workbench/contrib/webview/browser/webview';
import { IOverlayWebview, IWebviewService, WebviewExtensionDescription, WebviewInitInfo } from 'vs/workbench/contrib/webview/browser/webview';
import { PreviewWebview } from 'vs/workbench/contrib/positronPreview/browser/previewWebview';
import { IViewsService } from 'vs/workbench/services/views/common/viewsService';
import { POSITRON_PREVIEW_URL_VIEW_TYPE, POSITRON_PREVIEW_VIEW_ID } from 'vs/workbench/contrib/positronPreview/browser/positronPreviewSevice';
Expand Down Expand Up @@ -161,6 +161,7 @@ export class PositronPreviewService extends Disposable implements IPositronPrevi
options: {
enableFindWidget: true,
retainContextWhenHidden: true,
externalUri: this.canPreviewExternalUri()
},
contentOptions: {
allowScripts: true,
Expand All @@ -172,7 +173,7 @@ export class PositronPreviewService extends Disposable implements IPositronPrevi
};

const webview = this._webviewService.createWebviewOverlay(webviewInitInfo);
const preview = new PreviewUrl(previewId, webview, uri);
const preview = this.createPreviewUrl(previewId, webview, uri);

// Remove any other preview URLs from the item list; they can be expensive
// to keep around.
Expand All @@ -190,6 +191,28 @@ export class PositronPreviewService extends Disposable implements IPositronPrevi
return preview;
}

/**
* Creates a URL preview instance.
*
* @param previewId The preview ID
* @param webview The overlay webview instance
* @param uri The URI to open in the preview
* @returns A PreviewUrl instance
*/
protected createPreviewUrl(previewId: string, webview: IOverlayWebview, uri: URI): PreviewUrl {
return new PreviewUrl(previewId, webview, uri);
}

/**
* Indicates whether external URIs can be natively previewed in the viewer.
* Defaults to false; overridden to true in the Electron implementation.
*
* @returns True if external URIs can be previewed in the viewer; false otherwise
*/
protected canPreviewExternalUri(): boolean {
return false;
}

openPreviewWebview(
preview: PreviewWebview,
preserveFocus?: boolean | undefined
Expand Down
Loading
Loading