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

feat(browser,ui): port to Qt WebEngine #1125

Merged
merged 1 commit into from Apr 26, 2020
Merged

Conversation

kaygdev
Copy link
Contributor

@kaygdev kaygdev commented Aug 7, 2019

Porting from the obsolete QWebKit to the QWebEngine. Most features should work with it (QWebEngine does not have all features like QWebKit).

In addition, this modification blocks requests from local pages to web pages. Some documentation sets track the user with Google Analytics, this is now blocked.

@kaygdev kaygdev requested a review from trollixx as a code owner August 7, 2019 22:16
@trollixx
Copy link
Member

trollixx commented Sep 5, 2019

I would really like adding support for Qt WebEngine, but without losing backend Qt WebKit at least as a build option. Refactoring out the browser component was partially motivated by the idea of having different web view engines. The work is not finished yet, and I think here we can identify the missing pieces, and add necessary abstractions to accommodate both engines.

@kaygdev Would you be interested in continuing to work on this PR to land it as an option, rather than a replacement?

I see the need for QWeb(Engine)Settings to be encapsulated within the component. Another to-do item would be breaking direct inheritance of Browser::WebView from QWebView. More things might come up along the way.

@kaygdev
Copy link
Contributor Author

kaygdev commented Dec 4, 2019

Hi trollixx,

Sorry for the late response. I can change the replacement of QWebKit to a build option. However, this will take some time, because I currently have absolutely no time. I hope/think that this will change in spring.

@trollixx
Copy link
Member

trollixx commented Feb 7, 2020

Build time option is what Zeal used to have a while back, when QWebEngine was quite unstable. I believe it's much better now, but I don't want to drop support for Qt WebKit. Also it would be really great to use web engine provided by the platform on Windows, and that require having a proper abstraction layer.

I'll close this for now, feel free to reopen or create a new PR whenever you get time to work on this. It'd would also be better to discuss the design beforehand. Thanks!

@trollixx trollixx closed this Feb 7, 2020
@trollixx trollixx reopened this Apr 20, 2020
@trollixx
Copy link
Member

After spending more time figuring out how to deal with dual backend, and testing out this PR, I think it'd be much better to ship the next version of Zeal with Qt WebEngine rather than wait on the updated Qt WebKit. Once the next QtWK is released we can reassess the situation, and add it as the second backend. I think developing and abstraction layer will also be easier with WK2 based QtWK.

@trollixx
Copy link
Member

@kaygdev I'll rebase your changes on top of the current master, and merge.

@trollixx trollixx force-pushed the QWebEngine branch 9 times, most recently from 0a0d058 to 59fc0bb Compare April 20, 2020 14:44
@kaygdev
Copy link
Contributor Author

kaygdev commented Apr 20, 2020

Hi trollixx,

thanks for accepting and rebasing my pull request. You can contact me when a new version of Webkit is available. I hope I have time then and can support you with the implementation.

filter: invert(1) hue-rotate(180deg) contrast(120%) !important;
}

::-webkit-scrollbar {
Copy link
Member

Choose a reason for hiding this comment

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

I think scroll bars should follow the system theme not web page, will remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem with this was, in dark mode I have a complete dark window but with bright scrollbars in the docset part. The WebEngine don't use the the scrollbar style from the Qt Application.

Copy link
Member

Choose a reason for hiding this comment

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

I see. OK, let's keep this for now. In general I think the dark mode should be smarter and follow application's theme.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, it's a bit broken, no buttons. I'll look into styling scroll bars later.

Copy link
Member

@trollixx trollixx Apr 26, 2020

Choose a reason for hiding this comment

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

Lacks buttons on both ends:

image

Choose a reason for hiding this comment

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

I have always steadfastly refused to use ::-webkit-scrollbar styling because it mangles the scrollbar in this way. Hopefully they’ll follow Firefox’s lead soon, in implementing scrollbar-color and the rest of that lot, and in defaulting to a dark scrollbar (without ruining the controls) on dark content.

@trollixx trollixx changed the title Porting from QWebKit to QWebEngine feat(browser,ui): port to Qt WebEngine Apr 26, 2020
@trollixx trollixx merged commit 5360da3 into zealdocs:master Apr 26, 2020
@trollixx trollixx added this to the 0.7.0 milestone Apr 27, 2020
@asouini
Copy link

asouini commented May 1, 2020

This also fixes #865

tested this ci build https://ci.appveyor.com/project/zealdocs/zeal/builds/32459448 on a windows surface

@trollixx
Copy link
Member

trollixx commented May 1, 2020

@asouini thanks for reporting!

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