View Issue Details

IDProjectCategoryView StatusLast Update
0015404ScribusGeneralpublic2019-02-28 09:02
Reporterjsardid Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version1.5.4 
Summary0015404: Allow for KDE open/save dialogs in KDE sessions
DescriptionUnlike all Qt apps that I am aware of, Scribus is not using KDE open/save dialogs in a KDE session. I thought this was something automatic, but apparently not. This affects usability of Scribus for KDE users. I wonder if there is an easy way to make this possible as in other Qt applications.
Thanks.
Steps To ReproduceOpen a file in Scribus or save current document in a KDE session
Additional InformationUsing Scribus from Ubuntu PPA repository (scribus-ng 1.5.4-0ubuntu18.04~ppa0)
TagsNo tags attached.
PatchNo

Activities

jsardid

2018-08-19 13:20

reporter   ~0045400

Reproducible also in Scribus 1.5.5svn (scribus-trunk 1.5.5svn~r22635~20180819~ppa73~ubuntu18.04.1)

RJVB

2019-02-23 09:20

reporter   ~0045910

This is simply due to the fact that Scribus sets the `QFileDialog::DontUseNativeDialog` option (maybe to get the same dialog "experience" on all platforms?).

I'm attaching a PoC patch. A proper fix would probably expose the choice of dialog to use via the "User Interface" preferences (Libre/OpenOffice have done that for as long as I can remember).
patch-allow-kde-dialogs.diff (1,140 bytes)   
diff --git a/scribus/ui/newfile.cpp b/scribus/ui/newfile.cpp
index ec68e7404..7a3209cb5 100644
--- a/scribus/ui/newfile.cpp
+++ b/scribus/ui/newfile.cpp
@@ -366,7 +366,7 @@ void NewDoc::createOpenDocPage()
 	fileDialog = new QFileDialog(openDocFrame, tr("Open"), docDir, formats);
 	fileDialog->setFileMode(QFileDialog::ExistingFile);
 	fileDialog->setAcceptMode(QFileDialog::AcceptOpen);
-	fileDialog->setOption(QFileDialog::DontUseNativeDialog);
+// 	fileDialog->setOption(QFileDialog::DontUseNativeDialog);
 	fileDialog->setIconProvider(new ImIconProvider());
 	fileDialog->setNameFilterDetailsVisible(false);
 	fileDialog->setReadOnly(true);
diff --git a/scribus/ui/scfilewidget.cpp b/scribus/ui/scfilewidget.cpp
index f6577b980..589f6e08b 100644
--- a/scribus/ui/scfilewidget.cpp
+++ b/scribus/ui/scfilewidget.cpp
@@ -23,7 +23,7 @@ ScFileWidget::ScFileWidget(QWidget * parent) : QFileDialog(parent, Qt::Widget)
 {
 	m_forceDoubleClickActivation = false;
 
-	setOption(QFileDialog::DontUseNativeDialog);
+// 	setOption(QFileDialog::DontUseNativeDialog);
 	setSizeGripEnabled(false);
 	setModal(false);
 	setViewMode(QFileDialog::List);
patch-allow-kde-dialogs.diff (1,140 bytes)   

jghali

2019-02-23 14:43

administrator   ~0045911

>> This is simply due to the fact that Scribus sets the `QFileDialog::DontUseNativeDialog` option (maybe to get the same dialog "experience" on all platforms?).

This option is set because part of our file dialog explicitly requires using Qt dialog. By disabling this option, file dialog will be subject to issue 0011996.

jsardid

2019-02-23 15:36

reporter   ~0045913

I see why you did this, but using non-native dialogs in KDE is a pain in comparison to the native dialogs. I think any KDE user would say similarly.

One way to do it would be what GNU Octave has done in its version 5, setting an option for whether to use native dialogs or not:

Please see this bug for details: https://savannah.gnu.org/bugs/?54607

and the commit that fixed the issue: https://hg.savannah.gnu.org/hgweb/octave/rev/1002b49ce7b3

The default option could be set to "not using native dialogs", unless there is a way to check if the system is KDE. I don't recall if Octave does this automatically or I manually set the option to use native dialogs.

RJVB

2019-02-23 15:50

reporter   ~0045914

Indeed, I noticed that my PoC patch causes an issue on Mac, where the actual file dialog will open under instead of in the dialog put up by Scribus (let's call it the supposed parent). The result is that you can't use it because the supposed parent dialog is modal.
NB: many Mac users feel that applications should behave "as natively as possible" and thus use the platform file dialogs.

I plan to look into this because I *think* it should be possible to embed the actual file dialog into the supposed parent.

Alternatively, don't use Scribus file dialogs at all if the user elects to use native dialogs. You don't get the possibility to see previews (generated inside Scribus), but those are already optional (and the native dialogs might provide a similar feature).

RJVB

2019-02-23 16:56

reporter   ~0045916

Re: embedding: I probably confounded with KDE's KFileWidget *widget*.

You *could* of course use that (instead of the ScFileWidget, or make it a parent of that class?), though it means depending on one of the larger Tier3 frameworks.

The advantage of using this is that you get the same powerful file selection widget (offers a detailed tree view, for instance) on all supported platforms.

I'll try to find some time next week to tinker with making ScFileWidget inherit KFileWidget.

RJVB

2019-02-25 22:39

reporter   ~0045922

Here's a working patch that provides a file dialog that is based on `KFileWidget` from the KIO framework, which is core of the KDE file dialog. The patch is a bit rough around the edges; I haven't yet implemented the CMake logic that will allow the build to complete when KIO isn't available. (I've also not tried very hard to use the proper coding style.)

With this approach we get a very feature-rich file dialog that should work everywhere; screenshots made on Mac attached.

This patch was made against the 1.5.5 series, checked out from Git (#b0a8577cd8aee7702823b2c1b080bcc0db5dea54).

I'd love to hear from Scribus team members what they think of integrating this patch (and how much work it'd require...)
patch-use-kfilewidget.diff (23,809 bytes)   
diff --git a/scribus/ui/customfdialog.cpp b/scribus/ui/customfdialog.cpp
index 22da78bf3e8486589e88482245f284380856fb7e..574cc5039bba1e3250827d47e8a2d4570cc10678 100644
--- a/scribus/ui/customfdialog.cpp
+++ b/scribus/ui/customfdialog.cpp
@@ -493,7 +493,11 @@ CustomFDialog::CustomFDialog(QWidget *parent, const QString& wDir, const QString
 	connect(fileDialog, SIGNAL(filesSelected(const QStringList &)), this, SLOT(accept()));
 	connect(fileDialog, SIGNAL(accepted()), this, SLOT(accept()));
 	connect(fileDialog, SIGNAL(rejected()), this, SLOT(reject()));
+#ifndef SCFILEWIDGET_IS_KFILEWIDGET
 	resize(minimumSizeHint());
+#else
+    fileDialog->disablePreviewOption();
+#endif
 }
 
 
diff --git a/scribus/ui/newfile.cpp b/scribus/ui/newfile.cpp
index ec68e74047a47d6b15cbc321b477ff121fcef08d..3d0195049a7da5be7e93af3c23257c3322ad074a 100644
--- a/scribus/ui/newfile.cpp
+++ b/scribus/ui/newfile.cpp
@@ -363,10 +363,10 @@ void NewDoc::createOpenDocPage()
 	openDocLayout->setSpacing(5);
 	m_selectedFile = "";
 
-	fileDialog = new QFileDialog(openDocFrame, tr("Open"), docDir, formats);
+	fileDialog = new ScFileDialog(openDocFrame, tr("Open"), docDir, formats);
 	fileDialog->setFileMode(QFileDialog::ExistingFile);
 	fileDialog->setAcceptMode(QFileDialog::AcceptOpen);
-	fileDialog->setOption(QFileDialog::DontUseNativeDialog);
+// 	fileDialog->setOption(QFileDialog::DontUseNativeDialog);
 	fileDialog->setIconProvider(new ImIconProvider());
 	fileDialog->setNameFilterDetailsVisible(false);
 	fileDialog->setReadOnly(true);
@@ -376,8 +376,9 @@ void NewDoc::createOpenDocPage()
 	QListIterator<QPushButton *> i(b);
 	while (i.hasNext())
 		i.next()->setVisible(false);
-	fileDialog->setWindowFlags(Qt::Widget);
-	openDocLayout->addWidget(fileDialog);
+    auto dialog = dynamic_cast<QDialog*>(fileDialog);
+	dialog->setWindowFlags(Qt::Widget);
+	openDocLayout->addWidget(dialog);
 
 	FileDialogEventCatcher* keyCatcher = new FileDialogEventCatcher(this);
 	QList<QListView *> lv = fileDialog->findChildren<QListView *>();
@@ -390,9 +391,9 @@ void NewDoc::createOpenDocPage()
 	connect(keyCatcher, SIGNAL(homePressed()), this, SLOT(gotoHomeDirectory()));
 	connect(keyCatcher, SIGNAL(parentPressed()), this, SLOT(gotoParentDirectory()));
 	connect(keyCatcher, SIGNAL(enterSelectedPressed()), this, SLOT(gotoSelectedDirectory()));
-	connect(fileDialog, SIGNAL(currentChanged(const QString &)), this, SLOT(openFileDialogFileClicked(const QString &)));
-	connect(fileDialog, SIGNAL(filesSelected(const QStringList &)), this, SLOT(openFile()));
-	connect(fileDialog, SIGNAL(rejected()), this, SLOT(reject()));
+	connect(dialog, SIGNAL(currentChanged(const QString &)), this, SLOT(openFileDialogFileClicked(const QString &)));
+	connect(dialog, SIGNAL(filesSelected(const QStringList &)), this, SLOT(openFile()));
+	connect(dialog, SIGNAL(rejected()), this, SLOT(reject()));
 }
 
 void NewDoc::openFile()
diff --git a/scribus/ui/newfile.h b/scribus/ui/newfile.h
index d7aa0000131935f9251ca4c101bd0ea7babb6e44..39eb114f0a4007e20fe4de720c8175cffadc12a1 100644
--- a/scribus/ui/newfile.h
+++ b/scribus/ui/newfile.h
@@ -30,12 +30,20 @@ class QPushButton;
 #include "ui/customfdialog.h"
 #include "ui/nftwidget.h"
 
+#include "ui/scfilewidget.h"
+
 class PrefsManager;
 class MarginWidget;
 class ScrSpinBox;
 class ScComboBox;
 class QFileDialog;
 
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+using ScFileDialog = KFileDialog;
+#else
+using ScFileDialog = QFileDialog;
+#endif
+
 class SCRIBUS_API PageLayoutsWidget : public QListWidget
 {
 	Q_OBJECT
@@ -100,7 +108,7 @@ public:
 	ScrSpinBox* widthSpinBox;
 	ScrSpinBox* heightSpinBox;
 	QFrame* openDocFrame;
-	QFileDialog *fileDialog;
+	ScFileDialog *fileDialog;
 	QFrame* newFromTempFrame;
 	nftwidget *nftGui;
 	QFrame* recentDocFrame;
diff --git a/scribus/ui/scfilewidget.cpp b/scribus/ui/scfilewidget.cpp
index f6577b9806ca4ecad1fa5b83d6773cdde72b997d..0d2963d9ceaf10fd73191bdafa06f5398ecd86a8 100644
--- a/scribus/ui/scfilewidget.cpp
+++ b/scribus/ui/scfilewidget.cpp
@@ -18,18 +18,336 @@ for which a new license (GPL+exception) is in place.
 #include "filedialogeventcatcher.h"
 #include "scfilewidget.h"
 
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+#include <QLayout>
+#include <KDirOperator>
+#include <KFileFilterCombo>
+#include <KSharedConfig>
+#include <KWindowConfig>
+#include <QWindow>
+#endif
+
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+/*
+ * Map a list of Qt filter strings onto the KDE format.
+ */
+static QString qt2KdeFilter(const QStringList &f)
+{
+    QString               filter;
+    QTextStream           str(&filter, QIODevice::WriteOnly);
+    QStringList           list(f);
+    list.replaceInStrings(QStringLiteral("/"), QStringLiteral("\\/"));
+    QStringList::const_iterator it(list.constBegin()), end(list.constEnd());
+    bool                  first = true;
+
+    // map "description (*.ext1 *.ext2)" to "*.ext1 *.ext2|description"
+    for (; it != end; ++it) {
+        int ob = it->lastIndexOf(QLatin1Char('(')),
+            cb = it->lastIndexOf(QLatin1Char(')'));
+
+        if (-1 != cb && ob < cb) {
+            if (first) {
+                first = false;
+            } else {
+                str << '\n';
+            }
+            str << it->mid(ob + 1, (cb - ob) - 1) << '|' << it->mid(0, ob);
+        }
+    }
+
+    return filter;
+}
+
+/*
+ * Map a KDE filter string onto a Qt one from the list of known filter strings
+ */
+static QString kde2QtFilter(const QStringList &list, const QString &kde)
+{
+    QStringList::const_iterator it(list.constBegin()), end(list.constEnd());
+    int                   pos;
+
+    for (; it != end; ++it) {
+        if (-1 != (pos = it->indexOf(kde)) && pos > 0 &&
+                (QLatin1Char('(') == (*it)[pos - 1] || QLatin1Char(' ') == (*it)[pos - 1]) &&
+                it->length() >= kde.length() + pos &&
+                (QLatin1Char(')') == (*it)[pos + kde.length()] || QLatin1Char(' ') == (*it)[pos + kde.length()])) {
+            return *it;
+        }
+    }
+    return QString();
+}
+
+KFileDialog::KFileDialog(QWidget * parent, const QString &caption, const QString &directory, const QString &filter)
+    : QDialog(parent, Qt::Widget)
+    , m_sizeRestored(false)
+    , m_restoredSize(QSize(-1,-1))
+{
+    setLayout(new QVBoxLayout);
+    m_fileWidget = new KFileWidget(directory.isEmpty() ? QUrl() : QUrl::fromLocalFile(directory), this);
+    layout()->addWidget(m_fileWidget);
+    if (!caption.isEmpty()) {
+        setWindowTitle(caption);
+    }
+    if (!filter.isEmpty()) {
+        setNameFilter(filter);
+    }
+    connect(this, &KFileDialog::accepted, m_fileWidget, &KFileWidget::accept);
+    // sic, [accepted] "Should be used by the caller to call accept()."
+    connect(m_fileWidget, &KFileWidget::accepted, m_fileWidget, &KFileWidget::accept);
+    connect(this, &KFileDialog::rejected, m_fileWidget, &KFileWidget::slotCancel);
+    connect(m_fileWidget, &KFileWidget::accepted, this, &KFileDialog::accept);
+    connect(m_fileWidget, &KFileWidget::accepted, this, &KFileDialog::accept);
+    connect(m_fileWidget, &KFileWidget::fileHighlighted, this, &KFileDialog::fileHighlighted);
+    connect(m_fileWidget, &KFileWidget::accepted, this, &KFileDialog::slotFilesSelected);
+    connect(m_fileWidget, &KFileWidget::filterChanged, this, &KFileDialog::slotFilterSelected);
+    connect(m_fileWidget, &KFileWidget::accepted, this, &KFileDialog::saveSize);
+    connect(this, &KFileDialog::rejected, this, &KFileDialog::saveSize);
+    connect(this, &KFileDialog::accepted, this, &KFileDialog::saveSize);
+    m_fileWidget->dirOperator()->setAcceptDrops(true);
+    disablePreviewOption();
+    // save & restore dialog size but only if our parent is a QDialog
+    m_saveSize = qobject_cast<QDialog*>(parent);
+    restoreSize();
+}
+
+KFileDialog::~KFileDialog()
+{
+    saveSize();
+}
+
+void KFileDialog::restoreSize()
+{
+    if (!m_sizeRestored && m_saveSize) {
+        winId();
+        KSharedConfig::Ptr conf = KSharedConfig::openConfig();
+        KWindowConfig::restoreWindowSize(windowHandle(), conf->group("FileDialogSize"));
+        m_sizeRestored = true;
+        m_restoredSize = windowHandle()->size();
+        resize(windowHandle()->size());
+    }
+}
+
+void KFileDialog::saveSize()
+{
+    if (m_saveSize) {
+        KSharedConfig::Ptr conf = KSharedConfig::openConfig();
+        KConfigGroup group = conf->group("FileDialogSize");
+        KWindowConfig::saveWindowSize(windowHandle(), group);
+        m_sizeRestored = false;
+    }
+}
+
+void KFileDialog::show()
+{
+    restoreSize();
+    QDialog::show();
+}
+
+int KFileDialog::exec()
+{
+    restoreSize();
+    return QDialog::exec();
+}
+
+void KFileDialog::setAcceptMode(QFileDialog::AcceptMode mode)
+{
+    m_fileWidget->setOperationMode(mode == QFileDialog::AcceptOpen ? KFileWidget::Opening : KFileWidget::Saving);
+}
+
+QSize KFileDialog::sizeHint() const
+{
+    return m_restoredSize.isValid() ? m_restoredSize : m_fileWidget->dialogSizeHint();
+}
+
+QSize KFileDialog::minimumSizeHint() const
+{
+    return sizeHint();
+}
+
+QDir KFileDialog::directory() const
+{
+    return QDir(m_fileWidget->baseUrl().toLocalFile());
+}
+
+void KFileDialog::setDirectory(const QString &path)
+{
+    m_fileWidget->setUrl(QUrl::fromLocalFile(path));
+}
+
+void KFileDialog::setDirectory(const QDir &path)
+{
+    m_fileWidget->setUrl(QUrl::fromLocalFile(path.absolutePath()));
+}
+
+void KFileDialog::selectFile(const QString &path)
+{
+    QUrl dirUrl = QUrl::fromLocalFile(path).adjusted(QUrl::RemoveFilename);
+    m_fileWidget->setUrl(dirUrl);
+    m_fileWidget->setSelectedUrl(path);
+}
+
+void KFileDialog::setFileMode(QFileDialog::FileMode mode)
+{
+    KFile::Modes kmode = KFile::LocalOnly;
+    switch(mode) {
+        case QFileDialog::AnyFile:
+            kmode |= KFile::File;
+            break;
+        case QFileDialog::ExistingFile:
+            kmode |= KFile::File | KFile::ExistingOnly;
+            break;
+        case QFileDialog::Directory:
+            kmode |= KFile::Files | KFile::Directory;
+            break;
+        case QFileDialog::ExistingFiles:
+            kmode |= KFile::Files | KFile::ExistingOnly;
+            break;
+        case QFileDialog::DirectoryOnly:
+            kmode |= KFile::Directory;
+            break;
+    }
+    m_fileWidget->setMode(kmode);
+}
+
+QFileDialog::FileMode KFileDialog::fileMode() const
+{
+    auto kmode = m_fileWidget->mode();
+    // remove the LocalOnly flag
+    kmode &= ~KFile::LocalOnly;
+    switch (kmode) {
+        default:
+            qWarning() << Q_FUNC_INFO << "unforeseen KFileWidget::mode" << kmode;
+        case KFile::File:
+            return QFileDialog::AnyFile;
+            break;
+        case KFile::File | KFile::ExistingOnly:
+            return QFileDialog::ExistingFile;
+            break;
+        case KFile::Files | KFile::Directory:
+            return QFileDialog::Directory;
+            break;
+        case KFile::Files | KFile::ExistingOnly:
+            return QFileDialog::ExistingFiles;
+            break;
+        case KFile::Directory:
+            return QFileDialog::DirectoryOnly;
+            break;
+    }
+}
+
+void KFileDialog::setNameFilter(const QString &filter)
+{
+    if (filter.isEmpty()) {
+        return;
+    }
+    const auto list = filter.split(QStringLiteral(";;"));
+    if (list.isEmpty()) {
+        return;
+    }
+    if (!m_nameFilters.contains(filter)) {
+        m_nameFilters.append(filter);
+    }
+    auto fWidget = m_fileWidget->filterWidget();
+    fWidget->setFilter(qt2KdeFilter(list));
+    fWidget->setCurrentFilter(fWidget->filters().at(0));
+}
+
+void KFileDialog::selectNameFilter(const QString &filter)
+{
+    if (m_nameFilters.contains(filter)) {
+        const auto listQt = filter.split(QStringLiteral(";;"));
+        // User can try to select a composed filter, in which case we don't
+        // really know if s/he wants something other than the 1st filter
+        // in that list. Just take the 1st.
+        const auto f = qt2KdeFilter(QStringList(listQt.at(0)));
+        m_fileWidget->filterWidget()->setCurrentFilter(f);
+    } else {
+        qWarning() << Q_FUNC_INFO << "ignoring unknown filter" << filter;
+    }
+}
 
-ScFileWidget::ScFileWidget(QWidget * parent) : QFileDialog(parent, Qt::Widget)
+QStringList KFileDialog::nameFilters() const
+{
+    return m_nameFilters;
+}
+
+QString KFileDialog::selectedNameFilter() const
+{
+    auto fWidget = m_fileWidget->filterWidget();
+    if (fWidget->currentFilter().isEmpty()) {
+        // this shouldn't happen anymore (happens when calling setCurrentFilter() with
+        // a composite/multi filter pattern instead of with the 1st element from that pattern).
+        qWarning() << Q_FUNC_INFO << "empty filter! Known=" << fWidget->filters();
+        if (fWidget->filters().isEmpty()) {
+            fWidget->setFilter(tr("*"));
+        } else {
+            fWidget->setCurrentFilter(fWidget->filters().at(0));
+        }
+    }
+    return kde2QtFilter(m_nameFilters, fWidget->currentFilter());
+}
+
+void KFileDialog::setNameFilterDetailsVisible(bool)
+{
+}
+
+void KFileDialog::fileHighlighted(const QUrl &url)
+{
+    emit currentChanged(url.toLocalFile());
+}
+
+void KFileDialog::slotFilesSelected()
+{
+    emit filesSelected(m_fileWidget->selectedFiles());
+}
+
+void KFileDialog::slotFilterSelected(const QString &)
+{
+    const QString f = selectedNameFilter();
+//     qWarning() << Q_FUNC_INFO << filter << "\nvs." << f;
+    emit filterSelected(f);
+    reload();
+}
+
+void KFileDialog::reload()
+{
+    m_fileWidget->dirOperator()->rereadDir();
+    m_fileWidget->dirOperator()->updateDir();
+}
+
+void KFileDialog::disablePreviewOption()
+{
+    m_fileWidget->dirOperator()->setPreviewWidget(nullptr);
+}
+
+// unsupported/non-implemented
+void KFileDialog::setIconProvider(QFileIconProvider*) {}
+void KFileDialog::setReadOnly(bool) {}
+
+#endif
+
+#ifndef SCFILEWIDGET_IS_KFILEWIDGET
+ScFileWidget::ScFileWidget(QWidget * parent)
+    : QFileDialog(parent, Qt::Widget)
+#else
+ScFileWidget::ScFileWidget(QWidget *parent
+                           , const QString &caption
+                           , const QString &directory, const QString &filter
+                          )
+    : KFileDialog(parent, caption, directory, filter)
+#endif
 {
 	m_forceDoubleClickActivation = false;
 
-	setOption(QFileDialog::DontUseNativeDialog);
 	setSizeGripEnabled(false);
 	setModal(false);
-	setViewMode(QFileDialog::List);
 	setWindowFlags(Qt::Widget);
+#ifndef SCFILEWIDGET_IS_KFILEWIDGET
+	setOption(QFileDialog::DontUseNativeDialog);
+	setViewMode(QFileDialog::List);
 
-#ifdef Q_OS_MAC
+#ifdef Q_OS_MACOS
+    // no point with the KDE widget!
 	QList<QUrl> urls;
 	QUrl computer(QUrl::fromLocalFile(QLatin1String("")));
 	if (!urls.contains(computer))
@@ -41,9 +359,11 @@ ScFileWidget::ScFileWidget(QWidget * parent) : QFileDialog(parent, Qt::Widget)
 	if (!urls.contains(dt))
 		urls << dt;
 	setSidebarUrls(urls);
+#endif
 #endif
 
-	FileDialogEventCatcher* keyCatcher = new FileDialogEventCatcher(this);
+    // TODO: do we need this when using the KFileDialog?
+    FileDialogEventCatcher* keyCatcher = new FileDialogEventCatcher(this);
 	QList<QListView *> lv = findChildren<QListView *>();
 	QListIterator<QListView *> lvi(lv);
 	while (lvi.hasNext())
@@ -61,8 +381,12 @@ ScFileWidget::ScFileWidget(QWidget * parent) : QFileDialog(parent, Qt::Widget)
 		i.next()->setVisible(false);
 	setMinimumSize(QSize(480, 310));
 	setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Preferred);
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+    reload();
+#endif
 }
 
+
 void ScFileWidget::forceDoubleClickActivation(bool force)
 {
 	// Hack to make the previews in our file dialogs useable again,
@@ -73,17 +397,24 @@ void ScFileWidget::forceDoubleClickActivation(bool force)
 
 	if (force)
 		setStyleSheet(QStringLiteral("QAbstractItemView { activate-on-singleclick: 0; }"));
-	else
+    else
 		setStyleSheet(QString());
 	m_forceDoubleClickActivation = force;
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+    m_fileWidget->dirOperator()->setOnlyDoubleClickSelectsFiles(force);
+#endif
 }
 
 QString ScFileWidget::selectedFile()
 {
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+    return m_fileWidget->selectedFile();
+#else
 	QStringList l(selectedFiles());
 	if (l.count() == 0)
 		return QString();
 	return l.at(0);
+#endif
 }
 
 void ScFileWidget::locationDropped(const QString& fileUrl)
@@ -104,14 +435,18 @@ void ScFileWidget::locationDropped(const QString& fileUrl)
 
 void ScFileWidget::gotoParentDirectory()
 {
+#ifndef SCFILEWIDGET_IS_KFILEWIDGET
 	QDir d(directory());
 	d.cdUp();
 	setDirectory(d);
+#else
+    m_fileWidget->dirOperator()->cdUp();
+#endif
 }
 
 void ScFileWidget::gotoSelectedDirectory()
 {
-	QStringList s(selectedFiles());
+	QStringList s(m_fileWidget->selectedFiles());
 	if (!s.isEmpty())
 	{
 		QFileInfo fi(s.first());
diff --git a/scribus/ui/scfilewidget.h b/scribus/ui/scfilewidget.h
index 7ee83382f784cb3c2f5ffd7ce4b733cd3f1e5e81..d4d3b09cad832607bd6d7bd83e4bae69c9962f25 100644
--- a/scribus/ui/scfilewidget.h
+++ b/scribus/ui/scfilewidget.h
@@ -8,9 +8,82 @@ for which a new license (GPL+exception) is in place.
 #define SCFILEWIDGET_H
 
 #include <QFileDialog>
+#include <KFileWidget>
 
 #include "scribusapi.h"
 
+#define SCFILEWIDGET_IS_KFILEWIDGET
+class QFileIconProvider;
+
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+class SCRIBUS_API KFileDialog : public QDialog
+{
+    Q_OBJECT
+public:
+	KFileDialog(QWidget * parent
+        , const QString &caption = QString()
+        , const QString &directory = QString(), const QString &filter = QString()
+    );
+    virtual ~KFileDialog();
+
+    void setAcceptMode(QFileDialog::AcceptMode mode);
+    QSize sizeHint() const;
+    QSize minimumSizeHint() const;
+    QDir directory() const;
+    void setDirectory(const QString &path);
+    void setDirectory(const QDir &path);
+    void selectFile(const QString &path);
+    QStringList selectedFiles() const { return m_fileWidget->selectedFiles(); }
+    void setIconProvider(QFileIconProvider*);
+    void setReadOnly(bool);
+    void setFileMode(QFileDialog::FileMode mode);
+    QFileDialog::FileMode fileMode() const;
+    void setNameFilter(const QString &filter);
+    void selectNameFilter(const QString &filter);
+    QStringList nameFilters() const;
+    QString selectedNameFilter() const;
+    // obsolete
+    void setNameFilterDetailsVisible(bool);
+
+    // overloads to target the appropriate parent class
+    template<typename T>
+    QList<T> findChildren(const QString &aName = QString(), Qt::FindChildOptions options = Qt::FindChildrenRecursively) const
+    {
+        return m_fileWidget->findChildren<T>(aName, options);
+    }
+
+    void show();
+
+    void reload();
+    void disablePreviewOption();
+
+signals:
+    void currentChanged(const QString &path);
+    void filesSelected(const QStringList &selected);
+    void filterSelected(const QString &filter);
+
+public slots:
+    virtual int exec();
+
+private slots:
+    void fileHighlighted(const QUrl &url);
+    void slotFilesSelected();
+    void slotFilterSelected(const QString &filter);
+    void saveSize();
+
+private:
+    void restoreSize();
+    KFileWidget *m_fileWidget;
+    QStringList m_nameFilters;
+    bool m_sizeRestored;
+    bool m_saveSize;
+    QSize m_restoredSize;
+    friend class ScFileWidget;
+};
+
+using FileWidgetParentClass = KFileDialog;
+#endif
+
 /*! \brief Simple wrapper simulating the QFileDialog as a common widget.
 Usage is simple - everything is like in QFileDialog except it is not
 a dialog, it's a widget. It can be used for extension features etc.
@@ -22,12 +95,17 @@ user e.g. doubleclicks the "open" file (in the parent (new) dialog):
 connect(fileWidget, SIGNAL(accepted()), this, SLOT(accept()));
 \author Petr Vanek <petr@scribus.info>
 */
-class SCRIBUS_API ScFileWidget : public QFileDialog
+class SCRIBUS_API ScFileWidget : public FileWidgetParentClass
 {
 	Q_OBJECT
 
 public:
-	ScFileWidget(QWidget * parent);
+	ScFileWidget(QWidget * parent
+#ifdef SCFILEWIDGET_IS_KFILEWIDGET
+        , const QString &caption = QString()
+        , const QString &directory = QString(), const QString &filter = QString()
+#endif
+    );
 
 	QString selectedFile();
 	void forceDoubleClickActivation(bool force);
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 305e11b06797f209999e5d64463803ba2873e285..d372bfb806087f45304ff65bab4ad9d0ed8f0329 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -50,7 +50,10 @@ include(GNUInstallDirs)
 #enable_testing()
 
 #Set the custom CMake module directory where our include/lib finders are
-set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/modules")
+find_package(ECM NO_MODULE)
+
+set(SCRIBUS_CMAKE_MODULES "${CMAKE_SOURCE_DIR}/cmake/modules")
+set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH} ${ECM_KDE_MODULE_DIR} ${SCRIBUS_CMAKE_MODULES})
 
 ##############################################################################################################
 ########## toplevel compiler flags                                                                  ##########
@@ -705,6 +708,10 @@ message(STATUS "Qt VERSION: ${Qt5Widgets_VERSION}")
 #endif()
 #>> Qt
 
+find_package(KF5KIO)
+if(KF5KIO_FOUND)
+    message(STATUS "----- USE KF5::KIO -----")
+endif()
 
 #C++11 Support
 #as of 1.5.5.svn, require C++11
@@ -725,8 +732,8 @@ if (WIN32)
 	# On win32 we can use Qt's zlib and libpng, so we use some
 	# custom cmake includes. This permits us to honour the
 	# USE_QT_ZLIB_PNGLIB flag if passed.
-	set(PNG_DIR ${CMAKE_MODULE_PATH})
-	set(ZLIB_DIR ${CMAKE_MODULE_PATH})
+	set(PNG_DIR ${SCRIBUS_CMAKE_MODULES})
+	set(ZLIB_DIR ${SCRIBUS_CMAKE_MODULES})
 endif()
 find_package(ZLIB REQUIRED)
 if (ZLIB_FOUND)
@@ -805,7 +812,7 @@ if (RENDER_LIB)
 	endif()
 endif()
 
-set(CAIRO_DIR ${CMAKE_MODULE_PATH})
+set(CAIRO_DIR ${SCRIBUS_CMAKE_MODULES})
 find_package(CAIRO REQUIRED)
 if(CAIRO_FOUND)
 	message("CAIRO Library Found OK")
@@ -837,7 +844,7 @@ endif()
 #>>LibXML2
 
 #<<LittleCMS
-set(LCMS_DIR ${CMAKE_MODULE_PATH})
+set(LCMS_DIR ${SCRIBUS_CMAKE_MODULES})
 find_package(LCMS2 REQUIRED)
 if(LCMS2_FOUND)
 	set(HAVE_LCMS2 1)
@@ -851,7 +858,7 @@ endif()
 
 #<<FontConfig
 if(NOT WIN32)
-	set(FONTCONFIG_DIR ${CMAKE_MODULE_PATH})
+	set(FONTCONFIG_DIR ${SCRIBUS_CMAKE_MODULES})
 	find_package(FONTCONFIG)
 	if(FONTCONFIG_FOUND)
 		message("FontConfig Found OK")
@@ -904,7 +911,7 @@ endif()
 
 #<<GraphicksMagick for image import
 if (WANT_GRAPHICSMAGICK)
-	set(GMAGICK_DIR ${CMAKE_MODULE_PATH})
+	set(GMAGICK_DIR ${SCRIBUS_CMAKE_MODULES})
 	pkg_check_modules(GMAGICK GraphicsMagick)
 	# find_package(GMAGICK)
 	if(GMAGICK_FOUND)
diff --git a/scribus/CMakeLists.txt b/scribus/CMakeLists.txt
index c21ab492736b573b6ad4c28ba0bfc1d43650bd67..cb296f75cedc185102e56d21c415e5ecd15dbdf1 100644
--- a/scribus/CMakeLists.txt
+++ b/scribus/CMakeLists.txt
@@ -1293,6 +1293,11 @@ target_link_libraries(${EXE_NAME}
 	${WIN32_ONLY_LIBS}
 	${GMAGICK_LIBRARIES}
 )
+if(KF5KIO_FOUND)
+    target_link_libraries(${EXE_NAME}
+        KF5::KIOFileWidgets
+    )
+endif()
 
 if (NOT WIN32 AND NOT HAIKU)
 	target_link_libraries(${EXE_NAME}
diff --git a/scribus/plugins/scriptplugin/CMakeLists.txt b/scribus/plugins/scriptplugin/CMakeLists.txt
index bb3110572434c9c8ddbf08b5452bc344879289bc..08d7bdde17fb6d34e1f79b893fb387765e1f916d 100644
--- a/scribus/plugins/scriptplugin/CMakeLists.txt
+++ b/scribus/plugins/scriptplugin/CMakeLists.txt
@@ -72,6 +72,11 @@ target_link_libraries(${SCRIBUS_SCRIPTER_PLUGIN}
 )
 
 target_link_libraries(${SCRIBUS_SCRIPTER_PLUGIN} ${PLUGIN_LIBRARIES})
+if(KF5KIO_FOUND)
+    target_link_libraries(${SCRIBUS_SCRIPTER_PLUGIN}
+        KF5::KIOFileWidgets
+    )
+endif()
 
 install(TARGETS ${SCRIBUS_SCRIPTER_PLUGIN}
 	LIBRARY
patch-use-kfilewidget.diff (23,809 bytes)   

ale

2019-02-26 08:08

manager   ~0045923

in the current state, this patch is basically only useful for people who compile scribus on their machine.

but i fear that accepting it, will make some distribution force a dependency on (parts of) kde.
(i've seen it multiple time, that optional bindings to a DE have been made the default / mandatory)
personally, this is nothing i'd like to see happen.

personally, if some kde enhancement should happen, then it should be loaded at runtime not at compile time.

the real solution, imo, is to understand why scribus has issues with using the native dialogs, solve those issues, and use the native dialogs.

RJVB

2019-02-26 08:51

reporter   ~0045926

I don't disagree with the plugin idea, but implementing the basics of that goes way beyond what I can (and am willing to) contribute.

This patch does *not* introduce a dependency on a DE. The KDE frameworks have been turned into a set of more or less generic Qt extensions that for the most part are universally useful. Now if some "cheap" distribution decides to ship the frameworks only with the KDE desktop, why would that be an issue for Scribus (good chance they'd not be shipping a big advanced piece of software like Scribus ;))

At first glance the reason why Scribus doesn't work with the KDE/Plasma ("native") file dialog is because Scribus wants to use the dialog as a widget. That doesn't seem to work with the current Qt platform file dialog helper class and/or the way this is used in KDE's platform theme plugin (plasma-integration) ... or the way things are done in Scribus. That must be why you get a 2nd dialog.

RJVB

2019-02-26 10:16

reporter   ~0045927

I took the liberty:
https://bugs.kde.org/show_bug.cgi?id=404833

ale

2019-02-26 12:51

manager   ~0045928

scribus should not use the kde dialog if the desktop is not currently KDE.
and if i understand it correctly, your patch uses the KDE dialog if KDE IO is found at compile time, not at run time.

while it's true that the KDE framework has been turned into a set of things, as far i can tell, you cannot install one item all alone: you need a few of them... at least. and that few is normally rather big (in number and MBs).


but, i agree with you, that scribus should use the native dialog (when available) on gnome, kde, windows and mac... and provide the necessary information to the underlying system so that it just feels natural.

personally, i would prefer having no custom open / save dialogs in scribus.
there has been a patch made a few months ago that made the "raw" dialog work a bit better, but the scribus customizations are (for me) only a problem. (on my system file > open still does not work correctly... but it's not as broken as it used to be)

if scribus wants to show a preview (i think this is the main reason for the custom dialog) it should simply provide the needed information to the underlying system.
so, the first patch you uploaded is not that wrong in my eyes! (and that's the way i've patched my scribus for years!)

RJVB

2019-02-26 13:42

reporter   ~0045929

(So replying by email doesn't work with this tracker ... replies just disappear into a void?!)

>scribus should not use the kde dialog if the desktop is not currently KDE.

Regardless of whether or not you agree with that it *could* decide to use a file dialog that was originally written by KDE, simply because it suits its needs.
The KDE native platform file dialog is very powerful, but other platforms aren't so lucky (Gnome, Mac). Qt at least makes an effort to make QWidget-based dialogs look native.

>and if i understand it correctly, your patch uses the KDE dialog if KDE IO is
>found at compile time, not at run time.

Yes, because there's no way to do this runtime, currently. You'd need to refactor the code so it no longer uses QFileDialog but instead a wrapper class which can then use one of multiple mutually-exclusive plugin-based providers (something like `QFileDialog* ScFileDialogPlugin::createFileDialog()`).

>far i can tell, you cannot install one item all alone: you need a few of them...
>at least. and that few is normally rather big (in number and MBs).
 Yes, but Scribus itself is already very large, and in terms of pure binaries the additional footprint of adding just KIO and its dependencies shouldn't be an obstacle. Whether or not size is an issue is something that IMHO can be left safely to distro maintainers and those users who build themselves; official Scribus builds could offer both versions.

The way it's implemented now the possible use of KIO isn't even advertised as an option and no warning is printed when KIO isn't found. So package maintainers on non-KDE distributions might not even notice the optional dependency ;)

>but, i agree with you, that scribus should use the native dialog (when
>available) on gnome, kde, windows and mac... and provide the necessary

Actually I have mixed feelings about this. I used to think the same, but then I started missing more and more features in the native Mac dialog. At some point I figured out I could adapt KDE's platform integration plugin for Mac so I get properly themed applications (Qt's native Aqua theme makes apps look as if designed for the visually and motor impaired when you don't do all kinds of specific tweaks). That got me the KDE platform file dialog too, and now I just don't want to go back. The only regret I have is that the Mac directory-selection dialog is more powerful/convenient.

>if scribus wants to show a preview (i think this is the main reason for the
>custom dialog) it should simply provide the needed information to the underlying
>system.

But not all platforms provide preview functionality, and AFAIK Qt has no hooks for that either. The KDE KFileWidget does, but that requires writing a KDE-based preview provider. It seems that the default preview feature knows how to handle at least some Scribus files though (and Scribus's own preview fails on compressed Scribus files...)
But this is not all: the save dialog adds its own set of widgets.

>so, the first patch you uploaded is not that wrong in my eyes! (and that's the
>way i've patched my scribus for years!)

Maybe you should try my patch and see how things work for you that way :)
I install KF5 from source so it's hard for me to tell how much space just KIO and its deps use, but if you don't have them yet you can answer that question easily by installing KIO and KIO-dev or however your distro calls them.

ale

2019-02-26 14:15

manager   ~0045930

i've tried to add kio through the package manager: it pulls in 100 other packages.
100 more packages that i will regularly need to update.
(and, probably, a few services that will be started and will keep on running behind the scene)
no, thanks.

i really understand that you like the way KDE does things.
and you are 100% right to like it.
just as i like to have a rather simple (but mostly incredibly fast) computer.

really, if i did want to use the KDE enhancements, i would be using KDE
if i would have preferred the Gnome ones, i would be with Gnome.
Mac OS? of course, i could have bought one.
Windows? Why not?

but i don't want a KDE dialog on gnome. nor on Mac OS... nor on my simple tiny system.
and i'm pretty sure that there would be an outcry from so many mac users, if on Mac OS scribus started to use non standard dialogs.

i really believe that all those type of system have their reason for being the way they are and i really try not to mix them up.

yes, if you have chosen to use KDE, you should deserve to get a KDE dialog if this is possible (and in this case it is!)
and if somebody has chosen to use Gnome, she should deserve to get the Gnome dialogs she likes!
(and i deserve an open dialog that works correctly, as the Qt guys planned it. not the crippled one i get now!)

it's not up to scribus (no the person who compiles it) to decide which dialog you should get. it's you who should choose. the person who runs it.
and, currently, the right way to choose it is by going with what provides the user's platform.

my conclusion:

- scribus should revert to the native dialog
- have the needed preview providers for the most common system (afaik, they're rather easy to write)
- no idea how many people use the compression (and if i would suggest anybody to use it... the size of the .sla is mostly insignificant compared to the images in a scribus project).

but it's not me who decides.

RJVB

2019-02-26 16:57

reporter   ~0045931

> and i'm pretty sure that there would be an outcry from so many mac users, if on Mac OS scribus started to use non standard dialogs.

It already does. Mac native dialogs cannot be embedded either, they're subject to the same issue as KDE native dialogs, and for the same reason. It's very likely that the same happens on MS Windows.

> - have the needed preview providers for the most common system (afaik, they're rather easy to write)

Easy to write maybe, easy to provide in a single dialog together with the file selection, maybe (probably) not so much, if at all possible. You'd have to use preview popups, probably.

> the right way to choose it is by going with what provides the user's platform

You mean that the user doesn't get a choice at all beyond what control s/he has over the platform, which may be much more limited than you think. To me that's not good enough. Again, I agree that a plugin approach would be better, and I'd happily try to provide one - but I simply cannot justify the investment it'd take to develop the require architecture ... and get it accepted.

Some more screenshots, taken in Qt's Mac/native mode this time.

- Native file dialog in Qt's Standard Dialogs example
- Qt's "non-native" dialog in the same app. Looks like a Mac-theme on a random other platform, right? That's exactly what it is in fact, except that the theming on the widgets comes from Mac SDKs.
- the KIO-based file dialog in Scribus (2x). Did I mention that the Mac theme gives an awkward look if not hand-tuned? Note that I didn't bother setting up access to an icon theme here, hence the text-only toolbar.

RJVB

2019-02-27 10:23

reporter   ~0045932

@alo, could this suggestion (unset Qt::Dialog) be useful for you?

ale

2019-02-28 09:02

manager   ~0045933

sorry, rjvb, i don't really understand what your suggestion is...

all i want is:

- being able to type a url in the input line (or somewhere else) and get the autocomplete to work correctly.
- being able to click on the directories and, then, on .sla files.

what i am comfortable with:

- if the current "platform" offers more, use it! (without adding further dependencies for those who do not use that platform).

what i do not want:

- kde libraries when my DE is not kde.
- scribus customizing the dialog for little benefit and breaking common standard behaviors.

if you want to customize your experience when working on a mac, you should (imo) look for ways to do so on the system level, not by having the programs implement your preferred DE's behavior on all platforms.

qt is trying hard to copy the behavior of the host system, and this is something i like.

Issue History

Date Modified Username Field Change
2018-08-19 13:16 jsardid New Issue
2018-08-19 13:20 jsardid Note Added: 0045400
2019-02-23 09:20 RJVB File Added: patch-allow-kde-dialogs.diff
2019-02-23 09:20 RJVB Note Added: 0045910
2019-02-23 14:43 jghali Note Added: 0045911
2019-02-23 15:36 jsardid Note Added: 0045913
2019-02-23 15:50 RJVB Note Added: 0045914
2019-02-23 16:56 RJVB Note Added: 0045916
2019-02-25 22:39 RJVB File Added: patch-use-kfilewidget.diff
2019-02-25 22:39 RJVB File Added: Screen Shot 2019-02-25 at 23.35.39.png
2019-02-25 22:39 RJVB File Added: Screen Shot 2019-02-25 at 23.37.10.png
2019-02-25 22:39 RJVB Note Added: 0045922
2019-02-26 08:08 ale Note Added: 0045923
2019-02-26 08:51 RJVB Note Added: 0045926
2019-02-26 10:16 RJVB Note Added: 0045927
2019-02-26 12:51 ale Note Added: 0045928
2019-02-26 13:42 RJVB Note Added: 0045929
2019-02-26 14:15 ale Note Added: 0045930
2019-02-26 16:57 RJVB File Added: Screen Shot 2019-02-26 at 17.40.42.png
2019-02-26 16:57 RJVB File Added: Screen Shot 2019-02-26 at 17.41.24.png
2019-02-26 16:57 RJVB File Added: Screen Shot 2019-02-26 at 17.43.02.png
2019-02-26 16:57 RJVB File Added: Screen Shot 2019-02-26 at 17.43.56.png
2019-02-26 16:57 RJVB Note Added: 0045931
2019-02-27 10:23 RJVB Note Added: 0045932
2019-02-28 09:02 ale Note Added: 0045933