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

Do not serialize webview if the serializer for the webview is not set #8473

Closed
wants to merge 1 commit into from

Conversation

vitaliy-guliy
Copy link
Contributor

Signed-off-by: Vitaliy Guliy vgulyy@redhat.com

What it does

Disables saving of the webview state if the plugin don't register an apropriate theia.WebviewPanelSerializer

Original downstream issue eclipse-che/che#16352

How to test

Option 1:

  • write a plugin, which opens a webview but don't register an appropriate theia.WebviewPanelSerializer

Option 2 - Eclipse Che specific:

Option 3:

  • will think about it

Review checklist

Reminder for reviewers

Signed-off-by: Vitaliy Guliy <vgulyy@redhat.com>
widget.storeState = () => {
if (this.webviewRevivers.has(widget.viewType)) {
return storeState();
}
return {};

return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need it?

An idea was that {} object means don't restore, so a widget gets disposed in restoreState.

@akosyakov
Copy link
Member

What should I do to reproduce it on master?

@akosyakov akosyakov added 🤔 needs more info issues that require more info from the author webviews issues related to webviews labels Sep 9, 2020
Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

It would be nice to have an easy way for a reviewer to test the proposed patch.

@vitaliy-guliy
Copy link
Contributor Author

As it appeared, I cannot reproduce the issue on vanilla Theia.

For that I prepared simple extension, that adds a webview each time on load.
Binary of built extension could be loaded here https://github.com/vitaliy-guliy/welcome-extension/releases/download/0.0.1/welcome-extension-0.0.1.vsix.

When the extension adds vscode.WebviewPanelSerializer, we have +1 welcome page on each refresh the browser.
It's expected behavior, the extension must take into account that webview is restored and don't open it again.

But without vscode.WebviewPanelSerializer, the webview should not be persisted.

Let me check it again in Che-Theia. It could be because we use specific commit to initialize che-theia, not the latest commit form master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤔 needs more info issues that require more info from the author webviews issues related to webviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants