-
-
Notifications
You must be signed in to change notification settings - Fork 791
[Qt] Add a WebView widget. #3974
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
Conversation
This implements all the key features, but: - QtWebEngineWidgets is part of PySide6-Addons, so need to add the dependency - When loading html from a string, it looks like Qt uses a data URL with the content of the page, so when testing for the URL, we need to look at that. - Catching JavaScript errors is a bit of a hassle, and the current version of the code catches *all* error messages while the script runs, not just those from the running script. There doesn't seem to be any way to get something more fine-grained. - Cookie handling requires watching for cookies as they arrive, and adding them to a cookie jar one. The implementation copies the contents into a new jar when asked for the coookies.
freakboy3742
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another great PR. I've added the required screenshot, and a tweak to address the AddOns requirement; but otherwise, I think this is good to go.
Some other responses to comments in the PR body.
When loading html from a string, it looks like Qt uses a data URL with the content of the page, so when testing for the URL, we need to construct that. This appears to be a quirk on macOS.
We don't officially support Qt on macOS; it's nice that it does work, but it's not worth going to extraordinary levels to support edge cases there.
Catching JavaScript errors is a bit of a hassle, and the current version of the code catches all error messages while the script runs, not just those from the running script. There doesn't seem to be any way to get something more fine-grained.
Agreed that's less than ideal; but the perfect is the enemy of the good. What you've got here is good enough for now.
Cookie handling requires watching for cookies as they arrive, and adding them to a cookie jar one. The implementation copies the contents into a new jar when asked for the coookies.
It seems a little weird that Qt doesn't expose current state of the cookie jar; but the workaround you've got here sees fine to me.
I get segfaults in the testbed about 1 out of every 10 runs on macOS, hopefully in a more controlled CI environment it will be more stable, but it's possible there is a subtle error somewhere.
That's a little concerning; I guess we'll find out in time if it's a problem.
qt/pyproject.toml
Outdated
| [project.optional-dependencies] | ||
| pyside6 = [ | ||
| "PySide6-Essentials == 6.10.1", | ||
| "PySide6-Addons == 6.10.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged that this is needed for WebView support; but "add ons" is a big package to have as a hard requirement for apps that won't need it. I'd suggest adding (and documenting) a separate optional dependency group for pyside6-essentials that only includes the essentials.
FYI -- this is probably a testing issue going on here. We get consistent segfaults if we move add Though -- worth noting that our tests are on a separate thread than the main things. Does that make it kind of unsafe to maniuplate UI objects from non-main thread? |
Yes, it is unsafe - this has historically been, and will continue to be, a source of hard-crash errors. Even garbage collection can be an issue. The garbage collector should be able to run on any thread (and if Python is the only concern, it's no problem if it does); however, if it runs on a non-GUI thread, that can result in a "GUI action" on a non-GUI thread, which is a problem in many GUI toolkits. The testbed has a number of protections about this to ensure that GC doesn't run on non-GUI threads (see the number of calls to |
This implements all the key features.
Implementation notes:
When loading html from a string, it looks like Qt uses a data URL with the content of the page, so when testing for the URL, we need to construct that.This appears to be a quirk on macOS.Ref #3914.
To Do:
PR Checklist: