NotebookController asWebviewUri does not work for notebooks #121981

roblourens · 2021-04-23T02:24:38Z

Run a remote-ssh notebook that does something like this:

let remoteUri = controller.asWebviewUri(vscode.Uri.file('/datadrive/vscode-notebook-testbed/img.png'));
task.appendOutput(new vscode.NotebookCellOutput([ new vscode.NotebookCellOutputItem('text/html', `<html><img src=${remoteUri!.toString()} /></html>`) ]));

I get a uri https://0.vscode-webview-test.com/vscode-resource/file///datadrive/vscode-notebook-testbed/img.png, and it 404s

cc @mjbvz @rebornix @jrieken @DonJayamanne

The text was updated successfully, but these errors were encountered:

roblourens · 2021-04-23T02:41:31Z

Problem 1 - asWebviewUri needs the webview ID here:

vscode/src/vs/workbench/api/common/extHostNotebookKernels.ts

Line 173 in f68a7e7

return asWebviewUri(that._initData.environment, String(handle), uri);

Which makes me wonder whether NotebookController is the right place for this function now, since it's not tied to one editor. Or it needs to take a NotebookDocument from the extension too.

Problem 2
If I fix that, it fails here because extensionLocation is undefined:

vscode/src/vs/workbench/contrib/webview/browser/resourceLoading.ts

Line 133 in 595c14a

    
           if (resource.scheme === Schemas.file && extensionLocation?.scheme === Schemas.vscodeRemote) {

I don't know what to do about that because this webview is not associated with one extension. Maybe instead of extensionLocation it should use the root that was selected by if (containsResource(root, requestUri)) { which will be a remote URI

jrieken · 2021-04-23T13:42:19Z

Which makes me wonder whether NotebookController is the right place for this function now, since it's not tied to one editor. Or it needs to take a NotebookDocument from the extension too.

Does the ID have a meaning? Does it matches against some implicit webview id that notebook editors assign? I only reviewed how the internal asWebviewUri function looks and then went with the simple approach - using the controller handle instead of the cumbersome editor id. I might have missed the crucial part here... Tho, the selected controller is likely playing into this as well, e.g when I have two controllers with each having preloads then I should likely reload the webview when switching controllers

mjbvz · 2021-04-23T19:43:11Z

@jrieken Yes the id in asWebviewUri must be the same id that the webview itself uses

For webview panels, we create the webview id on the extension host side and then send this over to the main process before creating the webview. The confusing part is that the id we create on the extension host side is also used as the handle

For notebooks, I'm not sure if this same model will work or not since a notebookController can talk about multiple different notebook instances, correct?

roblourens · 2021-04-23T20:28:27Z

Talked to Matt offline, here's a summary

Problem 1, the webview URI needs the webview ID
- This ID is put into the URI domain, and the per-webview service worker checks it when deciding to handle a request -
  
  vscode/src/vs/workbench/contrib/webview/browser/pre/service-worker.js
  
  Line 174 in 48387df
  
  if (requestUrl.origin === resourceOrigin && requestUrl.pathname.startsWith(resourceRoot + '/')) {
- This is a problem when you would have the same notebook content in two split editors. Might just hack around it for notebooks for now but eventually we need to understand whether there are bad implications in security, caching, something else in not using this ID
Problem 2
- I will add an option for the notebook to set that tells the webview to assume the resource will be remote in a remote window
- A real fix will involve having this information, like the real remote URI in the uri returned from asWebviewUri. Which is not how we normally do things but this is not content that the uri transformer can find

…121981

roblourens · 2021-04-24T01:19:33Z

Hey @mjbvz if you get a chance you could look at d5210de. I went with adding a flag for the notebook service worker to ignore the first part of the domain for requests it's intercepting. I think this is ok for now. I really don't like what the extension has to do to pass the editor to asWebviewUri. Will merge before Monday's build.

@DonJayamanne there should be nothing you have to do for this to work.

DonJayamanne · 2021-04-24T01:24:17Z

ere should be nothing you have to do for this to work.

Great, I'm also mostly done with adding support for Widgets with the new notebook controller changes.
Thanks guys.

roblourens · 2021-04-24T18:45:34Z

@DonJayamanne if you want to test it, there's a windows build here, you could test with local or desktop remote at least, https://az764295.vo.msecnd.net/insider/d5210de428a4ed692c52cc598573833ec8c60040/VSCode-win32-x64-1.56.0-insider.zip lmk if you need a different platform

DonJayamanne · 2021-04-24T19:16:59Z

I'll need a mac build, thanks

roblourens · 2021-04-24T19:32:32Z

https://az764295.vo.msecnd.net/insider/d5210de428a4ed692c52cc598573833ec8c60040/VSCode-darwin.zip

DonJayamanne · 2021-04-24T20:27:44Z

Brilliant, works now, Please feel free to close this issue.

roblourens · 2021-04-24T20:37:20Z

Will keep it for the real fixes

DonJayamanne · 2021-04-24T20:37:57Z

Thanks, I've published the insider version of the Juypter exension to marketplace as well.

DonJayamanne · 2021-04-24T20:38:23Z

Will test on codespaces on monday.

jrieken · 2021-04-26T12:50:28Z

@roblourens I am very much in favour of not requiring the editor-id/instance when calling asWebviewUri. I think it makes the API much harder to work with because you are now in the business of tracking editors. The id is arbitrary but it needs to used symmetrically when creating the web view. I believe using the controller id makes sense because controllers define preloads and when changing controllers we should also reload the webview - at least when the old controller uses preloads that are different from the "new" preloads. Thinking about it: maybe a hash of the preloads is the best id...

jrieken · 2021-04-26T12:57:17Z

related unload discussion: #120747

…121981

…icrosoft#121981

roblourens · 2021-04-26T17:46:53Z

Opened #122221 (nice issue number) to continue

roblourens added bug Issue identified by VS Code Team member as probable bug notebook labels Apr 23, 2021

roblourens added this to the April 2021 milestone Apr 23, 2021

roblourens self-assigned this Apr 23, 2021

roblourens changed the title ~~NotebookController asWebviewUri does not work in remote~~ NotebookController asWebviewUri does not work for notebooks Apr 24, 2021

roblourens added a commit that referenced this issue Apr 24, 2021

Implement workaround for jupyter in codespaces for asWebviewUri issue #…

e6b8600

…121981

roblourens added a commit that referenced this issue Apr 24, 2021

Implement workaround for jupyter in codespaces for asWebviewUri issue #…

d5210de

…121981

DonJayamanne mentioned this issue Apr 24, 2021

NotebookController.asWebviewUri does not return a valid Uri #121866

Closed

roblourens modified the milestones: April 2021, May 2021 Apr 24, 2021

roblourens assigned mjbvz Apr 24, 2021

roblourens added a commit that referenced this issue Apr 26, 2021

Implement workaround for jupyter in codespaces for asWebviewUri issue #…

7b0a817

…121981

roblourens modified the milestones: May 2021, April 2021 Apr 26, 2021

roblourens added the important Issue identified as high-priority label Apr 26, 2021

gjsjohnmurray pushed a commit to gjsjohnmurray/vscode that referenced this issue Apr 26, 2021

Implement workaround for jupyter in codespaces for asWebviewUri issue m…

b076e2d

…icrosoft#121981

roblourens mentioned this issue Apr 26, 2021

NotebookController asWebviewUri issues for notebooks #122221

Closed

roblourens closed this as completed Apr 26, 2021

rebornix added the verified Verification succeeded label Apr 30, 2021

github-actions bot locked and limited conversation to collaborators Jun 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NotebookController asWebviewUri does not work for notebooks #121981

NotebookController asWebviewUri does not work for notebooks #121981

roblourens commented Apr 23, 2021 •

edited

Loading

roblourens commented Apr 23, 2021 •

edited

Loading

jrieken commented Apr 23, 2021 •

edited

Loading

mjbvz commented Apr 23, 2021

roblourens commented Apr 23, 2021

roblourens commented Apr 24, 2021 •

edited

Loading

DonJayamanne commented Apr 24, 2021

roblourens commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

roblourens commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

roblourens commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

jrieken commented Apr 26, 2021 •

edited

Loading

jrieken commented Apr 26, 2021

roblourens commented Apr 26, 2021

NotebookController asWebviewUri does not work for notebooks #121981

NotebookController asWebviewUri does not work for notebooks #121981

Comments

roblourens commented Apr 23, 2021 • edited Loading

roblourens commented Apr 23, 2021 • edited Loading

jrieken commented Apr 23, 2021 • edited Loading

mjbvz commented Apr 23, 2021

roblourens commented Apr 23, 2021

roblourens commented Apr 24, 2021 • edited Loading

DonJayamanne commented Apr 24, 2021

roblourens commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

roblourens commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

roblourens commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

DonJayamanne commented Apr 24, 2021

jrieken commented Apr 26, 2021 • edited Loading

jrieken commented Apr 26, 2021

roblourens commented Apr 26, 2021

roblourens commented Apr 23, 2021 •

edited

Loading

roblourens commented Apr 23, 2021 •

edited

Loading

jrieken commented Apr 23, 2021 •

edited

Loading

roblourens commented Apr 24, 2021 •

edited

Loading

jrieken commented Apr 26, 2021 •

edited

Loading