Skip to content

Conversation

@just-some-entity
Copy link

Added support for webview and changed qArgc in launch.cpp from 0 to 1 since webview expects QCoreApplication to atleast have the program name as argument, otherwise aborts leading to a crash in quickshell.

… since webview expects QCoreApplication to atleast have the program name as argument, otherwise aborts leading to a crash in quickshell.
@outfoxxed
Copy link
Member

Changing qArgC is fine but I do not want to force a dependency on qtwebview or qtwebengine. Can we avoid calling initialize?

@just-some-entity
Copy link
Author

According to the docs: (https://doc.qt.io/qt-6/qml-qtwebview-webview.html#details)
To make the Qt WebView module function correctly across all platforms, it is necessary to call QtWebView::initialize() right after creating the QGuiApplicationinstance.

In https://doc.qt.io/qt-6/qtwebview.html they clarify that: This function initializes resources or sets options that are required by the different back-ends.

I tested it both with initialize() and without on my laptop and PC, and observed no visible issues. So it might work. However, there are no guarantees, and using it could lead to hard-to-debug undefined behavior when others start using or distributing configurations with WebView enabled. Since quickshell targets Linux only, I might review the QtWebView source for initialize() and try to implement it directly in qs, assuming the implementation is not too complex. This would avoid the dependency, though we would need to manually keep the code in sync with upstream.

@outfoxxed
Copy link
Member

if the initialization logic is complex, an acceptable solution would be to have a skeleton header with just that static function, and load the module dynamically if a pragma is set.

@just-some-entity
Copy link
Author

Alright. I removed the cmake options and added the pragma. When set to true it now calls the init function in webengine/webengine.hpp which attempts to load the appropriate library and symbol to call QtWebEngineQuick::initialize. Since webengine.hpp is only included by launch.cpp I figured offloading the function to a .cpp was not necessary.

@outfoxxed
Copy link
Member

Looks acceptable, though loading a symbol with a mangled name seems a little iffy https://doc.qt.io/qt-6/qlibrary.html#resolve. Can you add a manual test so we can make sure it still works in the future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants