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

VSCode extension with HTML part broken #6578

Closed
lemmy opened this issue Nov 19, 2019 · 21 comments
Closed

VSCode extension with HTML part broken #6578

lemmy opened this issue Nov 19, 2019 · 21 comments
Assignees
Labels
bug bugs found in the application vscode issues related to VSCode compatibility webviews issues related to webviews

Comments

@lemmy
Copy link

lemmy commented Nov 19, 2019

Description

The TLA+ VSCode extension has rendering issues with Theia and Theia fails completely with the extension's HTML part.

Gitpod with hash f5d7d65f-bdab-4f57-994b-3ad1539792bb was created from this TLA+ spec.

Reproduction Steps

Peek 2019-11-18 21-10

In VSCode

Peek 2019-11-18 21-15

/cc @alygin

@akosyakov akosyakov added the webviews issues related to webviews label Nov 19, 2019
@akosyakov
Copy link
Member

Assume that it should be fixed by #6465 Could you try please?

@akosyakov akosyakov added the vscode issues related to VSCode compatibility label Nov 19, 2019
@akosyakov akosyakov self-assigned this Nov 19, 2019
@akosyakov akosyakov added the bug bugs found in the application label Nov 19, 2019
@akosyakov
Copy link
Member

akosyakov commented Nov 19, 2019

with #6465:
Screen Shot 2019-11-19 at 09 17 31

It does not look good because the extension is not designed to work remotely. It uses vscode-resource scheme as a csp source: https://github.com/alygin/vscode-tlaplus/blob/f1fb18ef796181f42ab74b3fc53b2d976ba9edec/resources/check-result-view.html#L5

It does not work when resources are served from the remote http endpoint. It has to be fixed to use webview.cspSource as described in VS Code docs: https://code.visualstudio.com/api/extension-guides/webview#content-security-policy

@alygin
Copy link

alygin commented Nov 20, 2019

@akosyakov, thanks for the clarification. You're right, the TLA+ extension still uses the old (pre-1.38) WebView API and has rather strict security policies. Here's the issue about upgrading: tlaplus/vscode-tlaplus#111

Switching to the new API is an easy task, but unfortunately it still doesn't make the extension work in Theia. It looks like Theia doesn't provide this new API and throws the "viewPanel.webview.asWebviewUri is not a function" error. Any plans on supporting it?

There's another option -- work with the old API in the extension, but ease the security policies. It might help with loading the resources, but I would keep it as a last resort.

@akosyakov
Copy link
Member

akosyakov commented Nov 20, 2019

Switching to the new API is an easy task, but unfortunately it still doesn't make the extension work in Theia. It looks like Theia doesn't provide this new API and throws the "viewPanel.webview.asWebviewUri is not a function" error. Any plans on supporting it?

@alygin not on master, PR to properly support WebViews is here: #6465 I can try if provide vsix file with the fix.

@alygin
Copy link

alygin commented Nov 20, 2019

Sure, here it is:
vscode-tlaplus-1.2.1.vsix.zip

@akosyakov
Copy link
Member

akosyakov commented Nov 20, 2019

@alygin I've tried, it does not work for me, looking at the code, this part looks suspicious:

const resourcesDiskPath = vscode.Uri.file(path.join(extContext.extensionPath, 'resources'));
    const resourcesPath = viewPanel.webview.asWebviewUri(resourcesDiskPath);
    if (!viewHtml) {
        viewHtml = fs.readFileSync(path.join(resourcesPath.fsPath, 'check-result-view.html'), 'utf8');
    }
    

asWebviewUri returns an external URI to load local content, in remote context it is something like http://webview-host/vscode-resource/file/.... The point is that you cannot read filesystem with that and you should not. The extension is always running on the workspace machine and you can just do fs.readFileSync(path.join(path.join(extContext.extensionPath, 'resources', 'check-result-view.html')), 'utf8');.

Important that within the webview resources should be loaded using links created by asWebviewUri.

@akosyakov
Copy link
Member

And if i replace reading html file from the disk, it works 🎉
Screen Shot 2019-11-20 at 11 40 40

@alygin
Copy link

alygin commented Nov 20, 2019

...The point is that you cannot read filesystem with that and you should not...

Yeah, I didn't notice that. Thanks!

@alygin
Copy link

alygin commented Nov 20, 2019

@akosyakov, there's one problem left -- source file links are not displayed in the result view. Could you, please, click the "Full output" link and send me the contents of the output channel that will open? I'll try to figure out how to fix those links.

@alygin
Copy link

alygin commented Nov 20, 2019

@akosyakov, sorry, never mind, I managed to obtain it myself with the old extension version.

@alygin
Copy link

alygin commented Nov 20, 2019

@akosyakov, it looks like the problem is with css-styles. For instance, there's the table named Coverage on the panel. The Action column of this table contains links ("Init" and "Next" are links), yet they are displayed as simple text. Could you, please, check them? Is it really a style-related problem?

@akosyakov
Copy link
Member

@alygin I will check it tomorrow!

@alygin
Copy link

alygin commented Nov 21, 2019

@akosyakov, please, also have a look at those "M" symbols to the right of some of the error trace tree items. They have a css-style with the color var(--vscode-gitDecoration-modifiedResourceForeground) (should be blue in the default color scheme), yet it's displayed as a simple text.

There're other similar css-styles that don't work. The mentioned variables come from the Theme Color reference.

@akosyakov
Copy link
Member

akosyakov commented Nov 21, 2019

@alygin missing var(--vscode-gitDecoration-modifiedResourceForeground) is expected, Theia does not come with built-in VS Code extensions by default, and this color is contributed by the Git VS Code extension. It should work with #6475 though given that the Git VS Code extension is installed.

Anyway, thank you for spotting it!

@alygin
Copy link

alygin commented Nov 21, 2019

Got it, thanks!

Then the only problem left is with link styles. It's strange that some links are displayed correctly ("Full output", "Hide/Show unmodified" ), and others aren't.

The only difference I see is that those links that are displayed correctly, have the "title" attribute.

@akosyakov
Copy link
Member

@alygin I've inspected these elements and they don't look like a tags, they are td elements:
Screen Shot 2019-11-21 at 14 33 07

I'm not sure that it is Theia issue. We don't mutate HTML content, except rewriting old vscode-resource URLs to external URLs.

@akosyakov
Copy link
Member

akosyakov commented Nov 21, 2019

While debugging something strange happening here:
Screen Shot 2019-11-21 at 14 40 11

item.range is an object, but treated as an array

It's a bug on our side, see #6353 (comment)

@alygin
Copy link

alygin commented Nov 21, 2019

@akosyakov, you've spotted it correctly, appendCodeLinkChild() won't add the <a> element if the last parameter is empty.

I guess, this issue can be closed. All the problems are either fixed or covered in other specialised issues. @lemmy?

@lemmy
Copy link
Author

lemmy commented Nov 21, 2019

I don't know how to test the changes and thus are not qualified to comment.

@akosyakov
Copy link
Member

I will close the issue as soon as we merge #6465 It won't be available in Gitpod immediately, some integration work is required, but hopefully soon.

@akosyakov
Copy link
Member

General support in Theia resolved by #6465

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application vscode issues related to VSCode compatibility webviews issues related to webviews
Projects
None yet
Development

No branches or pull requests

3 participants