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

Use more stable hostname for webviews #13092 #13225

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Jan 3, 2024

What it does

  • add optional viewId to WebviewWidgetIdentifier, which may be used to create stable hostnames for webviews
  • use this viewId to store a uuid which will be used as part of the stable hostname

For VSCode the WebView iFrame src url looks like this:

vscode-webview://0a7iho5kovpr5p4ctn5fvcns9h4du51vr2fb4iachonmmqsceaes/fake.html?id=ad9dff6c-9d20-4aec-b8b1-edfeecc19d07 (initial open)
vscode-webview://0a7iho5kovpr5p4ctn5fvcns9h4du51vr2fb4iachonmmqsceaes/fake.html?id=faec60d8-1f93-4e66-94b7-931081a8a4ba (close and reopen; restart of application)

In Theia, the pattern was like this:

http://401ed951-431e-4866-9a42-b033d95521d4.webview.localhost:33391/webview/fake.html?id=401ed951-431e-4866-9a42-b033d95521d4 (initial open)
http://837d0c96-f06d-4bad-91ba-efec327876be.webview.localhost:33391/webview/fake.html?id=837d0c96-f06d-4bad-91ba-efec327876be (close and reopen)
http://6444a35c-161b-4205-96b4-a5aee01c0f9e.webview.localhost:37883/webview/fake.html?id=6444a35c-161b-4205-96b4-a5aee01c0f9e (restart of application)

The changing id was used as part of the hostname. Also the port changes between restarts of the electron app.

With this change the src url will look more like this:

http://33544e87-f49c-45eb-b7f7-3387c39fc913.webview.localhost:33391/webview/fake.html?id=401ed951-431e-4866-9a42-b033d95521d4 (initial open)
http://33544e87-f49c-45eb-b7f7-3387c39fc913.webview.localhost:33391/webview/fake.html?id=837d0c96-f06d-4bad-91ba-efec327876be (close and reopen)
http://33544e87-f49c-45eb-b7f7-3387c39fc913.webview.localhost:37883/webview/fake.html?id=6444a35c-161b-4205-96b4-a5aee01c0f9e (restart of application)

In order to fix the port in electron, which is 0 by default so any non-used port, adopers may use the --port option when starting Theia. I think 0 should stay the default.

Fixes #13092

How to test

Webviews should still work as before.
When it comes to testing the storage you may use the reproducer from #13092 and start the electron example either with

  • yarn electron start --port=3124
  • a custom electron main script, e.g.
process.argv.push('--port=3124')
require('../lib/backend/electron-main.js');

Follow-ups

Review checklist

Reminder for reviewers

* add optional viewId to WebviewWidgetIdentifier, which may be used to
create stable hostnames for webviews
* use this viewId to store a uuid which will be used as part of the
stable hostname
@jfaltermeier jfaltermeier marked this pull request as ready for review January 3, 2024 09:17
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Change looks good to me, thank you very much Johannes!

I can see that with a fixed port both the session storage and the local storage behave as in VS Code. Without a fixed port, only the session storage behaves as expected and the local storage is cleared. While I understand the technical reasons behind it, I am wondering whether there is a way to also abstract from the port number using a custom scheme as VS Code does or whether this is something we do not even want. In any case, this contribution already provides enough value in itself so it can definitely be merged.

@jfaltermeier jfaltermeier merged commit 72421be into master Jan 3, 2024
14 checks passed
@github-actions github-actions bot added this to the 1.46.0 milestone Jan 3, 2024
@jfaltermeier
Copy link
Contributor Author

jfaltermeier commented Jan 3, 2024

Thanks. I haven't investigated if we could introduce a custom scheme, but if required at some point we can come back to it.
For the browser version we may want to keep the current scheme though.

@jfaltermeier jfaltermeier deleted the jf/webview_id branch January 3, 2024 15:48
@msujew msujew added the webviews issues related to webviews label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
webviews issues related to webviews
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

vscode: incorrect Web API localStorage and sessionStorage behavior in Theia webview
3 participants