[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: serious kfiledialog bug
From:       Rafael =?iso-8859-1?q?Fern=E1ndez_L=F3pez?= <ereslibre () kde ! org>
Date:       2008-12-06 2:41:57
Message-ID: 200812060342.00657.ereslibre () kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


Hi,

I can undestand now bug #176657. The attached patch fixes the problem.

Basically what's going on is that KDirOperator doesn't know the context. It 
will emit that the file has been selected without knowing whether you are 
saving or loading.

This means that you always load or save with one or two clicks. You cannot 
load with one click, and save with two. KDirOperator doesn't get that far (at 
least until now).

At the same time KFileWidget only receives from KDirOperator the fileSelected() 
signal, without the context of what happened: one or two clicks on it (it 
could filter by using operationMode attribute...).

Basically there are two approaches of fixing this:

- We can give context to KDirOperator, so it will emit fileSelected if we click 
on a dir, or we click on a file when loading. It will emit fileSelected if we 
double click on a file and we are saving. It will filter the single click on a 
file if we are saving.

- We can add another signal to KDirOperator, so KFileWidget can find out if 
there happened single or double click on the current element.

I have written the first approach. This can be taken as possibly "harm?" for 
other apps that use KDirOperator internally. It shouldn't be a problem since 
the default is "loading", which will emit fileSelected the same times as 
before.

OK to commit ?


Regards,
Rafael Fernández López.

["kdelibs-kfile.diff" (text/x-patch)]

diff --git a/kfile/kdiroperator.cpp b/kfile/kdiroperator.cpp
index a3a701c..daeb5e3 100644
--- a/kfile/kdiroperator.cpp
+++ b/kfile/kdiroperator.cpp
@@ -217,7 +217,6 @@ public:
     void _k_slotRedirected(const KUrl&);
     void _k_slotProperties();
     void _k_slotPressed(const QModelIndex&);
-    void _k_slotClicked(const QModelIndex&);
     void _k_slotActivated(const QModelIndex&);
     void _k_slotDoubleClicked(const QModelIndex&);
     void _k_slotSelectionChanged();
@@ -284,6 +283,8 @@ public:
     bool showPreviews;
     int iconsZoom;
 
+    bool isSaving;
+
     KActionMenu *decorationMenu;
     KToggleAction *leftAction;
     KUrl::List itemsToBeSetAsCurrent;
@@ -313,6 +314,7 @@ KDirOperator::Private::Private(KDirOperator *_parent) :
     previewGenerator(0),
     showPreviews(false),
     iconsZoom(0),
+    isSaving(false),
     decorationMenu(0),
     leftAction(0),
     shouldFetchForItems(false),
@@ -902,6 +904,20 @@ int KDirOperator::iconsZoom() const
     return d->iconsZoom;
 }
 
+void KDirOperator::setIsSaving(bool isSaving)
+{
+    if (d->isSaving == isSaving) {
+        return;
+    }
+
+    d->isSaving = isSaving;
+}
+
+bool KDirOperator::isSaving() const
+{
+    return d->isSaving;
+}
+
 void KDirOperator::trashSelected()
 {
     if (d->itemView == 0) {
@@ -1537,6 +1553,8 @@ void KDirOperator::setView(QAbstractItemView *view)
 
     connect(d->itemView, SIGNAL(activated(const QModelIndex&)),
             this, SLOT(_k_slotActivated(const QModelIndex&)));
+    connect(d->itemView, SIGNAL(doubleClicked(const QModelIndex&)),
+            this, SLOT(_k_slotDoubleClicked(const QModelIndex&)));
     connect(d->itemView, SIGNAL(pressed(const QModelIndex&)),
             this, SLOT(_k_slotPressed(const QModelIndex&)));
     connect(d->itemView, SIGNAL(customContextMenuRequested(const QPoint&)),
@@ -2297,23 +2315,13 @@ void KDirOperator::Private::_k_slotProperties()
 void KDirOperator::Private::_k_slotPressed(const QModelIndex&)
 {
     // Remember whether the left mouse button has been pressed, to prevent
-    // that a right-click on an item opens an item (see _k_slotClicked(),
-    // _k_slotDoubleClicked() and _k_openContextMenu()).
+    // that a right-click on an item opens an item (see _k_slotDoubleClicked() and
+    // _k_openContextMenu()).
     const Qt::KeyboardModifiers modifiers = QApplication::keyboardModifiers();
     leftButtonPressed = (QApplication::mouseButtons() & Qt::LeftButton) &&
                         !(modifiers & Qt::ShiftModifier) && !(modifiers & Qt::ControlModifier);
 }
 
-void KDirOperator::Private::_k_slotClicked(const QModelIndex& index)
-{
-    if (!leftButtonPressed || (index.column() != KDirModel::Name)) {
-        return;
-    }
-
-    if (!parent->onlyDoubleClickSelectsFiles())
-        _k_slotDoubleClicked(index);
-}
-
 void KDirOperator::Private::_k_slotActivated(const QModelIndex& index)
 {
     const QModelIndex dirIndex = proxyModel->mapToSource(index);
@@ -2323,10 +2331,11 @@ void KDirOperator::Private::_k_slotActivated(const QModelIndex& index)
     if (item.isNull() || (modifiers & Qt::ShiftModifier) || (modifiers & Qt::ControlModifier))
         return;
 
-    if (item.isDir())
+    if (item.isDir()) {
         parent->selectDir(item);
-    else
+    } else if (!isSaving) {
         parent->selectFile(item);
+    }
 }
 
 void KDirOperator::Private::_k_slotDoubleClicked(const QModelIndex& index)
@@ -2356,7 +2365,7 @@ void KDirOperator::Private::_k_slotSelectionChanged()
 
     // In the multiselection mode each selection change is indicated by
     // emitting a null item. Also when the selection has been cleared, a
-    // null item must be emitted (see _k_slotClicked()).
+    // null item must be emitted.
     const bool multiSelectionMode = (itemView->selectionMode() == QAbstractItemView::ExtendedSelection);
     const bool hasSelection = itemView->selectionModel()->hasSelection();
     if (multiSelectionMode || !hasSelection) {
diff --git a/kfile/kdiroperator.h b/kfile/kdiroperator.h
index a7de1c1..0cde3c3 100644
--- a/kfile/kdiroperator.h
+++ b/kfile/kdiroperator.h
@@ -560,6 +560,21 @@ public:
      */
     int iconsZoom() const;
 
+    /**
+     * If the system is set up to trigger items on single click, if @p isSaving
+     * is true, we will force to double click to accept.
+     * @note this is false by default
+     * @since 4.2
+     */
+    void setIsSaving(bool isSaving);
+
+    /**
+     * Returns whether KDirOperator will force a double click to accept.
+     * @note this is false by default
+     * @since 4.2
+     */
+    bool isSaving() const;
+
 protected:
     /**
      * A view factory for creating predefined fileviews. Called internally by setView,
@@ -848,7 +863,6 @@ private:
     Q_PRIVATE_SLOT( d, void _k_slotRedirected(const KUrl&) )
     Q_PRIVATE_SLOT( d, void _k_slotProperties() )
     Q_PRIVATE_SLOT( d, void _k_slotPressed(const QModelIndex&) )
-    Q_PRIVATE_SLOT( d, void _k_slotClicked(const QModelIndex&) )
     Q_PRIVATE_SLOT( d, void _k_slotActivated(const QModelIndex&) )
     Q_PRIVATE_SLOT( d, void _k_slotDoubleClicked(const QModelIndex&) )
     Q_PRIVATE_SLOT( d, void _k_slotSelectionChanged() )
diff --git a/kfile/kfilewidget.cpp b/kfile/kfilewidget.cpp
index 2d57b6b..61ca5a8 100644
--- a/kfile/kfilewidget.cpp
+++ b/kfile/kfilewidget.cpp
@@ -375,6 +375,7 @@ KFileWidget::KFileWidget( const KUrl& _startDir, QWidget *parent )
 
     d->ops = new KDirOperator(KUrl(), d->opsWidget);
     d->ops->setObjectName( "KFileWidget::ops" );
+    d->ops->setIsSaving(d->operationMode == Saving);
     opsWidgetLayout->addWidget(d->ops);
     connect(d->ops, SIGNAL(urlEntered(const KUrl&)),
             SLOT(_k_urlEntered(const KUrl&)));
@@ -1869,6 +1870,10 @@ void KFileWidget::setOperationMode( OperationMode mode )
        d->okButton->setGuiItem( KStandardGuiItem::ok() );
     d->updateLocationWhatsThis();
     d->updateAutoSelectExtension();
+
+    if (d->ops) {
+        d->ops->setIsSaving(mode == Saving);
+    }
 }
 
 KFileWidget::OperationMode KFileWidget::operationMode() const

["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]