Notebook: hardcoded file assumptions

[bpasero]

The context is: #107437 and the deprecation of getPathFromAmdModule

I notice that src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts makes assumptions about using file protocol:

vscode/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts

Line 557 in ddc98c3

const rootPath = URI.file(path.dirname(getPathFromAmdModule(require, '')));

and a deprecated getPathFromAmdModule use:

vscode/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts

Line 358 in ddc98c3

const pathsPath = getPathFromAmdModule(require, 'vs/loader.js');

The new methods are FileAccess.asFileUri and FileAccess.asBrowserUri. It would be great if notebooks could adopt this accordingly.

[bpasero]

Just to clarify: currently on master these methods are not implemented fully, they basically just return the same URI. But once the vscode-file branch is merged, the asBrowserUri will return a vscode-file scheme while asFileUri ensures a file scheme.

[rebornix]

I use isWeb to decide whether to use asBrowserUri or asFileUri but correct me if I was wrong.

[bpasero]

Ok, let's reopen this issue to make sure we do the right fix, you bring up a good question and looking at your changes I am not entirely sure they are correct.

Trying to recap:

• asBrowserUri: this will be https in web and vscode-file in native once the vscode-file branch is merged to master (until then, the scheme will be file for native). This method should be used whenever you have Chrome load something (e.g. via DOM)
• asFileUri: as you probably already figured out only makes sense to be used in native context and never in web because this always returns a file:// URI. However, this can ONLY be used in contexts where node.js is used to access a file. As you can see here, we will start to block any access to file protocols that happen via a browser (e.g. renderer, webview, any DOM access)

I suggest that you give your changes a try from the vscode-file branch that will block any access to file protocol. That branch can just be run out of sources.

Since I am not an expert in the webview code there, maybe could also connect with @mjbvz to ensure the right thing is done.

[rebornix]

I checked vscode-file branch and it's working fine, I guess the main reason is we will convert all Uris to webview Uris like

vscode-webview-resource://b9b8afaf-8eaf-4a3a-935d-dd5557181017/file///Users/penlv/code/vscode/out/vs/loader.js

note that it's vscode-webview-resource://${GUID}/file/${path}, which means that webview component in main thread @mjbvz wrote will handle the file loading.

[bpasero]

Ping

[mjbvz]

@bpasero @rebornix Is there any more work needed on this issue?

[bpasero]

I think not from my end, looks like what I mentioned in #107438 (comment) has been addressed. If notebooks work fine when vscode-file:// protocol is enabled, feel free to close.

[mjbvz]

Closing this based on comments

I believe we also plan on removing the loader: #121245