[BUG]: "View website in reader mode" changes content of all tabs to its reader mode content

[dakralex]

Brief description of the issue

I have used the tab feature where I opened the links from one feed article in multiple tabs. If I click on any of these tabs and then press the "View website in reader mode", all the other tabs' content and the original article's content view is replaced with the reader view content of the tab I clicked the button at.

How to reproduce the bug?

1. Click on any feed
2. Click on any feed article (with links in it)
3. Open one or more links in a new tab
4. Switch to any of these newly created tabs
5. Click the "View website in reader mode" button
6. Change to any other tab
7. Assert that they have the content of the content of the reader mode of the previous tab

What was the expected result?

It was expected that the reader mode view would only apply to the current tab and won't change the content of the other tabs.

What actually happened?

The tab where the reader mode was requested changed the content of all the other tabs' content, which required to reopen them again (+ the original feed article).

Debug log

https://pastebin.com/8av6Ge0X (expires in 1 month)

Operating system and version

• OS: Linux 6.7.5-arch1-1 x86_64 GNU/Linux
• RSS Guard version: 4.6.3 (Build date: 12/15/23 3:23 PM)

[dakralex]

I truncated the debug log from the entries that included the downloaded links and replaced home folder names as well as external urls. I couldn't find an exact spot where the "View website in reader mode" incident happened, but I would guess somewhere in the 16-20 s ballpark.

If I find the time and/or someone could push me in the right direction where this is implemented in the application, I'm happy to contribute a fixing PR.

[dakralex]

My knowledge on Qt is limited as I have only ever read but never really written anything useful in Qt, but I guess since the qApp seems to be a singleton on the whole RSSGuard application instance, and the m_webFactory field seems to also be a singleton class instance, it seems like the bug is caused by replacing the web content in all WebViewers at the same time.

Here is the code that defines the action on the "View website in reader mode" button:

rssguard/src/librssguard/gui/webbrowser.cpp

Lines 32 to 34 in 31a598c

	m_actionReadabilePage(new QAction(qApp->icons()->fromTheme(QSL("text-html")),
	tr("View website in reader mode"),
	this)) {

rssguard/src/librssguard/gui/webbrowser.cpp

Line 83 in 31a598c

connect(m_actionReadabilePage, &QAction::triggered, this, &WebBrowser::readabilePage);

rssguard/src/librssguard/gui/webbrowser.cpp

Lines 164 to 167 in 31a598c

	void WebBrowser::readabilePage() {
	m_actionReadabilePage->setEnabled(false);
	qApp->web()->readability()->makeHtmlReadable(m_webView->html(), m_webView->url().toString());
	}

Here is the code that is run on a successful readabiled action (that is, the node-js module was already installed):

rssguard/src/librssguard/network-web/readability.cpp

Lines 111 to 144 in 31a598c

	QString temp_script =
	QDir::toNativeSeparators(IOFactory::getSystemFolder(QStandardPaths::StandardLocation::TempLocation)) +
	QDir::separator() + QSL("readabilize-article.js");
	if (!IOFactory::copyFile(QSL(":/scripts/readability/readabilize-article.js"), temp_script)) {
	qWarningNN << LOGSEC_ADBLOCK << "Failed to copy Readability script to TEMP.";
	}
	QProcess* proc = new QProcess(this);
	connect(proc,
	QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
	this,
	&Readability::onReadabilityFinished);
	qApp->nodejs()->runScript(proc, temp_script, {base_url});
	proc->write(html.toUtf8());
	proc->closeWriteChannel();
	}
	void Readability::onReadabilityFinished(int exit_code, QProcess::ExitStatus exit_status) {
	QProcess* proc = qobject_cast<QProcess*>(sender());
	if (exit_status == QProcess::ExitStatus::NormalExit && exit_code == EXIT_SUCCESS) {
	emit htmlReadabled(QString::fromUtf8(proc->readAllStandardOutput()));
	}
	else {
	QString err = QString::fromUtf8(proc->readAllStandardError());
	emit errorOnHtmlReadabiliting(err);
	}
	proc->deleteLater();
	}

When the readability signal/slot is finished, it is connected to the WebBrowser::setReadabledHtml() method

rssguard/src/librssguard/gui/webbrowser.cpp

Lines 90 to 91 in 31a598c

	connect(qApp->web()->readability(), &Readability::htmlReadabled, this, &WebBrowser::setReadabledHtml);
	connect(qApp->web()->readability(), &Readability::errorOnHtmlReadabiliting, this, &WebBrowser::readabilityFailed);

Which in turn is forwarded to one of the BrowserViewers (either TextBrowserViewer or WebEngineViewer). I guess since the Reader mode looks more like a Markdown viewer it will be the TextBrowserViewer (but I haven't checked) and maybe the bug lies somewhere in the TextBrowserViewer::setHtmlPrivate method?

rssguard/src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp

Lines 481 to 494 in 31a598c

	void TextBrowserViewer::setReadabledHtml(const QString& html, const QUrl& base_url) {
	setHtml(html, base_url);
	}
	void TextBrowserViewer::setHtmlPrivate(const QString& html, const QUrl& base_url) {
	m_currentUrl = base_url;
	m_currentHtml = html;
	QTextBrowser::setHtml(html);
	setZoomFactor(m_zoomFactor);
	emit pageTitleChanged(documentTitle());
	emit pageUrlChanged(base_url);
	}

[dakralex]

Okay, nevermind. Ater debugging I found that when clicking the "View website in reader mode" button, it will in fact call the WebEngineViewer::setHtml which forwards the call directly to the underlying QWebEngineView, which probably sets the html for all instance/the singleton instance. I guess this would need fixing.

rssguard/src/librssguard/gui/webviewers/webengine/webengineviewer.cpp

Lines 142 to 144 in 31a598c

	void WebEngineViewer::setHtml(const QString& html, const QUrl& base_url) {
	QWebEngineView::setHtml(html, base_url);
	}

[martinrotter]

@dakralex Your deduction is wrond. This given line simply calls "setHtml" method implementation from base class from which WebEngineViewer derives. No static/singleton-like is invoked.

That said, I am lately very very busy with my other projects but will look into all newly posted bugs/tickets eventually. Do not worry, RSS Guard is not dead.

[martinrotter]

OK, there really was problem. Pushed the fix. Please wait for devbuild to compile and test. Then let me know the result.

[dakralex]

@martinrotter Wow, I didn't expect such a timely answer and didn't think RSSGuard was dead. I also hope that my writing style didn't offend you in any way, I was just interested in how the program works, look into it and maybe even fix it myself. But as I said, I'm a newbie on the intrinsic functionality of Qt and were mostly just guessing in a short period of time.

I've just built the recent commit on my machine and it works, very nice! It works as expected now.

Nonetheless, I want to say to you that I enjoy RSSGuard very much, so thank you very much for maintaining it. If you have some Qt-beginner-friendly issues on here I can help you with, don't hesitate to send me them - I wanted to get into Qt anyway but haven't found a project yet :).

[martinrotter]

@dakralex No, not any problem my friend. Maybe phrasing my answer was too direct. No problems. Yes. This is is C++, just regular programming error I made, like hundreds of other errors in the app waiting for discovery. :D

[martinrotter]

As for any C++ maintenance of RSS Guard, well. Start with something small. First, learn how to compile RSS Guard (which is fairly simple). Then just try to tweak small things and see their impact. C++ has rather steep learning curve, even with Qt involved.

You can help with any opened issue you want. There are many issues which are relatively easy to solve but just outside of my time.

#1231

and many others.