Differential D17563 Diff 47595 src/articleviewer-ng/webengine/articleviewerwebengine.cpp

Changeset View

Standalone View

View Options

src/articleviewer-ng/webengine/articleviewerwebengine.cpp

Show All 40 Lines
41#include <KMessageBox> 41​#include <KMessageBox>
42#include <QPrinter> 42​#include <QPrinter>
43#include <QMouseEvent> 43​#include <QMouseEvent>
44#include <QApplication> 44​#include <QApplication>
45#include <QClipboard> 45​#include <QClipboard>
46#include <QMenu> 46​#include <QMenu>
47#include <viewerplugintoolmanager.h> 47​#include <viewerplugintoolmanager.h>
48#include <QWebEngineProfile> 48​#include <QWebEngineProfile>
49​#include <QWebEngineUrlRequestInterceptor>
49 50​
50#include <WebEngineViewer/WebHitTestResult> 51​#include <WebEngineViewer/WebHitTestResult>
51#include <WebEngineViewer/WebHitTest> 52​#include <WebEngineViewer/WebHitTest>
52#include <WebEngineViewer/WebEngineScript> 53​#include <WebEngineViewer/WebEngineScript>
53 54​
54#include <WebEngineViewer/LocalDataBaseManager> 55​#include <WebEngineViewer/LocalDataBaseManager>
55 56​
56#include <KIO/KUriFilterSearchProviderActions> 57​#include <KIO/KUriFilterSearchProviderActions>
57 58​
58using namespace Akregator; 59​using namespace Akregator;
59 60​
61​class AkregatorRequestInterceptor : public QWebEngineUrlRequestInterceptor
62​{
63​ Q_OBJECT
64​
65​public:
66​ explicit AkregatorRequestInterceptor(QObject *parent = nullptr) : QWebEngineUrlRequestInterceptor(parent) {
67​ }
68​
mlaurentUnsubmitted
Not Done

~AkregatorRequestInterceptor() override = default(); no ?

mlaurent: ~AkregatorRequestInterceptor() override = default(); no ?
martenUnsubmitted
Not Done

Is there any point in using 'override' on destructors? There is no function signature to trigger the usual warning case (derived class defines a function with a different signature, so not really replacing the base class virtual), so there is no way a destructor can be written to not override. The compiler doesn't consider destructors in its override warning.

The only way in which 'override' would be useful on a destructor would be to give an error if the base class destructor was not declared virtual, but with a correctly written base class this should not arise.

marten: Is there any point in using 'override' on destructors? There is no function signature to…
amantiaAuthorUnsubmitted
Not Done

I removed the destructor completely.

amantia: I removed the destructor completely.
kfunkUnsubmitted
Not Done

Sorry for getting more off-topic, but /me wants to clarify this:

The only way in which 'override' would be useful on a destructor would be to give an error if the base class destructor was not declared virtual, (...)

Right, and why not protect against this? I think adding an override to dtors makes a lot of sense. Especially if you think of override as of: "I expect there to exist a base class version of this function which is declared virtual".

Also see discussion on qt-development: https://lists.qt-project.org/pipermail/development/2018-August/033437.html

kfunk: Sorry for getting more off-topic, but /me wants to clarify this: > The only way in which…
69​ void interceptRequest(QWebEngineUrlRequestInfo &info) override {
70​ Q_UNUSED(info);
71​ }
72​};
73​
60ArticleViewerWebEngine::ArticleViewerWebEngine(KActionCollection *ac, QWidget *parent) 74​ArticleViewerWebEngine::ArticleViewerWebEngine(KActionCollection *ac, QWidget *parent)
61 : WebEngineViewer::WebEngineView(parent) 75​ : WebEngineViewer::WebEngineView(parent)
62 , mActionCollection(ac) 76​ , mActionCollection(ac)
63 , mLastButtonClicked(LeftButton) 77​ , mLastButtonClicked(LeftButton)
64 , mViewerPluginToolManager(nullptr) 78​ , mViewerPluginToolManager(nullptr)
65{ 79​{
66 mNetworkAccessManager = new WebEngineViewer::InterceptorManager(this, ac, this); 80​ mNetworkAccessManager = new WebEngineViewer::InterceptorManager(this, ac, this);
81​
kfunkUnsubmitted
Not Done

Note: You dont actually need a member at all here, no?

kfunk: Note: You dont actually need a member at all here, no?
amantiaAuthorUnsubmitted
Not Done

Right, it is possible without it as well.

amantia: Right, it is possible without it as well.
67 QWebEngineProfile *profile = QWebEngineProfile::defaultProfile(); 82​ QWebEngineProfile *profile = QWebEngineProfile::defaultProfile();
68 mPageEngine = new ArticleViewerWebEnginePage(profile, this); 83​ mPageEngine = new ArticleViewerWebEnginePage(profile, this);
69 profile->setPersistentCookiesPolicy(QWebEngineProfile::ForcePersistentCookies); 84​ profile->setPersistentCookiesPolicy(QWebEngineProfile::ForcePersistentCookies);
70 85​
86​ // Needed to workaround crash in webengine, see https://bugreports.qt.io/browse/QTBUG-72260
87​ auto webEngineUrlInterceptor = new AkregatorRequestInterceptor();
88​ profile->setRequestInterceptor(webEngineUrlInterceptor);
89​ connect(profile, &QObject::destroyed, webEngineUrlInterceptor, &AkregatorRequestInterceptor::deleteLater);
90​
71 setPage(mPageEngine); 91​ setPage(mPageEngine);
72 92​
73 connect(this, &ArticleViewerWebEngine::showContextMenu, this, &ArticleViewerWebEngine::slotShowContextMenu); 93​ connect(this, &ArticleViewerWebEngine::showContextMenu, this, &ArticleViewerWebEngine::slotShowContextMenu);
74 94​
75 setFocusPolicy(Qt::WheelFocus); 95​ setFocusPolicy(Qt::WheelFocus);
76 connect(mPageEngine, &ArticleViewerWebEnginePage::urlClicked, this, &ArticleViewerWebEngine::slotLinkClicked); 96​ connect(mPageEngine, &ArticleViewerWebEnginePage::urlClicked, this, &ArticleViewerWebEngine::slotLinkClicked);
77 97​
78 mWebEngineViewAccessKey = new WebEngineViewer::WebEngineAccessKey(this, this); 98​ mWebEngineViewAccessKey = new WebEngineViewer::WebEngineAccessKey(this, this);
▲ Show 20 Lines • Show All 410 Lines • ▼ Show 20 Line(s)
489{ 509​{
490 const QString text = selectedText(); 510​ const QString text = selectedText();
491 if (!text.isEmpty()) { 511​ if (!text.isEmpty()) {
492 interface->setText(text); 512​ interface->setText(text);
493 } 513​ }
494 interface->setUrl(mCurrentUrl); 514​ interface->setUrl(mCurrentUrl);
495 interface->execute(); 515​ interface->execute();
496} 516​}
517​
518​#include "articleviewerwebengine.moc"