Commit 779c1f43 authored by Jonathan Marten's avatar Jonathan Marten Committed by Stefano Crocco
Browse files

kfmclient: Fix recursive invocation and handle association correctly

If 'kcmshell5 componentchooser' is used to set the default web
browser to Konqueror, then as well as changing the text/html file
association it sets [General]BrowserApplication=kfmclient_html.desktop
in kdeglobals.  ClientApp::createNewWindow() looks to see whether this
setting is present, but actually ignores its value and simply opens
the original URL via an OpenUrlJob.  The file type association then
calls kfmclient again and loops endlessly.

Split up the complicated test to handle the two cases appropriately:
either launching the external browser (setting starting with a '!')
using a CommandLauncherJob, or starting the service (setting not
starting with '!') using an ApplicationLauncherJob.  Check in both
of these code paths that kfmclient would not be called recursively.
If this happens, or if the setting is absent or there are problems
launching the job, then fall through to opening the URL in Konqueror
directly.  This means that it cannot loop and the URL is opened in
the intended application.
parent e00322a6
......@@ -30,16 +30,16 @@
#include <kmessagebox.h>
#include <kmimetypetrader.h>
#include <kservice.h>
#include <KIO/OpenUrlJob>
#include <KIO/JobUiDelegate>
#include <KIO/CommandLauncherJob>
#include <KIO/ApplicationLauncherJob>
#include <KStartupInfo>
#include <kurifilter.h>
#include <KConfig>
#include <KConfigGroup>
#include <KJobWidgets>
#include <KService>
#include <KAboutData>
#include <KWindowSystem>
#include <KShell>
#include <kcoreaddons_version.h>
......@@ -48,14 +48,10 @@
#include <QDir>
#include <QMimeDatabase>
#include <QUrl>
#include <QStandardPaths>
#include <QCommandLineParser>
#include <QCommandLineOption>
#include <QTimer>
#include <QX11Info>
#ifdef WIN32
#include <process.h>
......@@ -181,30 +177,89 @@ bool ClientApp::createNewWindow(const QUrl &url, bool newTab, bool tempFile, con
KConfig config(QStringLiteral("kfmclientrc"));
KConfigGroup generalGroup(&config, "General");
const QString browserApp = generalGroup.readEntry("BrowserApplication");
if (!browserApp.isEmpty() && !browserApp.startsWith(QLatin1String("!kfmclient"))
&& (browserApp.startsWith('!') || KService::serviceByStorageId(browserApp))) {
qCDebug(KFMCLIENT_LOG) << "Using external browser" << browserApp;
KIO::OpenUrlJob *job = new KIO::OpenUrlJob(url);
//job->setUiDelegate(new KIO::JobUiDelegate(KJobUiDelegate::AutoHandlingEnabled, window));
QObject::connect(job, &KJob::result, this, [this](KJob *job) {
if (job->error()) {
if (!browserApp.isEmpty()) {
// There is a configured browser application.
// See whether it is a literal command (starting with '!')
// or a service (no '!').
if (browserApp.startsWith('!')) {
// A literal command. Split the string up into a shell
// command and arguments.
qCDebug(KFMCLIENT_LOG) << "Using external browser command" << browserApp;
QStringList shellArgs = KShell::splitArgs(browserApp.mid(1), KShell::AbortOnMeta);
if (!shellArgs.isEmpty()) {
// The command name is the first list item.
QString executable = shellArgs.takeFirst();
// Ensure that we are not calling ourselves recursively;
// that is, the external command is not "kfmclient" or
// any variation of it. If it is, then fall through to
// open the URL in Konqueror directly.
if (executable.startsWith("kfmclient")) {
goto launchKonq;
// Append the URL to be opened to the arguments.
// Then launch the command. It is not possible to automatically
// delete a temporary file in this case, but no temporary file
// download should have happened anyway.
auto *job = new KIO::CommandLauncherJob(executable, shellArgs);
QObject::connect(job, &KJob::result, this, &ClientApp::slotResult);
return qApp->exec();
} else {
qCWarning(KFMCLIENT_LOG) << "Parsing browser command failed";
} else {
// The configured browser is specified as a service.
qCDebug(KFMCLIENT_LOG) << "Using external browser service" << browserApp;
// First ensure that we are not calling ourselves recursively;
// that is, the service is not "kfmclient" or any variation
// of it. If it is, then fall through to open the URL in Konqueror
// directly.
if (browserApp.startsWith("kfmclient")) {
goto launchKonq;
KService::Ptr service = KService::serviceByStorageId(browserApp);
if (service) {
// Launch the service to open the URL.
auto *job = new KIO::ApplicationLauncherJob(service);
QObject::connect(job, &KJob::result, this, &ClientApp::slotResult);
if (tempFile) {
return qApp->exec();
} else {
qCWarning(KFMCLIENT_LOG) << "External browser service not known";
return qApp->exec();
// Fall through to here if the external browser command or service
// could not be found or run. Do not fall back to the original action
// of simply opening the specified URL in its default application,
// because if that turns out to be ourselves then there will again
// be an infinite recursion. Just exit with an error.
qCWarning(KFMCLIENT_LOG) << "Unable to launch external browser";
return false;
// The configured external browser command or service appears to be
// trying to call ourselves recursively. Simply fall through to
// launch Konqueror.
qCDebug(KFMCLIENT_LOG) << "Recursive external browser command or service detected";
// Launch Konqueror, or reuse an existing instance if possible.
KonqClientRequest req;
......@@ -223,7 +278,7 @@ void ClientApp::delayedQuit()
// Quit in 2 seconds. This leaves time for OpenUrlJob to pop up
// "app not found" in KProcessRunner, if that was the case.
QTimer::singleShot(2000, qApp, SLOT(quit()));
QTimer::singleShot(2000, qApp, &QApplication::quit);
static void checkArgumentCount(int count, int min, int max)
......@@ -288,13 +343,12 @@ bool ClientApp::doIt(const QCommandLineParser &parser)
return true;
void ClientApp::slotResult(KJob *job)
if (job->error() && m_interactive) {
KJobWidgets::setWindow(job, nullptr);
if (job->error()) {
} else {
const bool ok = !job->error();
qApp->exit(ok ? 0 : 1);
......@@ -42,7 +42,9 @@ public:
bool openProfile(const QString &profile, const QUrl &url, const QString &mimetype = QString());
private Q_SLOTS:
void slotResult(KJob *);
void slotResult(KJob *job);
void delayedQuit();
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment