File 0012-Unix-Fix-usability-of-the-print-properties... of Package libqt5-qtbase

Overview Repositories Revisions Requests Users Attributes Meta

File 0012-Unix-Fix-usability-of-the-print-properties-dialog.patch of Package libqt5-qtbase (Revision 615ef0f82aaa28ec93c6122d1e3957f9)

Currently displaying revision 615ef0f82aaa28ec93c6122d1e3957f9 , Show latest

From d94ccf310a9ca01593750a34f743ec652f6a344e Mon Sep 17 00:00:00 2001
From: Albert Astals Cid <albert.astals.cid@kdab.com>
Date: Wed, 27 Dec 2017 11:12:19 +0100
Subject: [PATCH 1/1] Unix: Fix usability of the print properties dialog

Previous behavior:
 * Open, change setting, cancel, open again, setting was as originally (i.e. unchanged)
 * Open, change setting, accept, open, change setting, cancel, open again, the setting would be as before pressing cancel
 * Open, change setting, accept, open, press cancel without changing anything, print, the initially changed setting is not applied

New behavior:
 * Pressing cancel just cancels the changes since you opened the dialog, everything you accepted previously stays correctly selected

Change-Id: I483647504682f26d3d21c5229cc6530bf14fe519
Reviewed-by: Andy Shaw <andy.shaw@qt.io>
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@qt.io>
---
 src/printsupport/dialogs/qpagesetupdialog_unix.cpp |  25 +++++
 src/printsupport/dialogs/qpagesetupdialog_unix_p.h |   6 ++
 src/printsupport/dialogs/qprintdialog_unix.cpp     | 107 +++++++++++++--------
 src/printsupport/kernel/qcups_p.h                  |  10 +-
 src/printsupport/widgets/qcupsjobwidget.cpp        |  25 ++++-
 src/printsupport/widgets/qcupsjobwidget_p.h        |   7 ++
 6 files changed, 136 insertions(+), 44 deletions(-)

diff --git a/src/printsupport/dialogs/qpagesetupdialog_unix.cpp b/src/printsupport/dialogs/qpagesetupdialog_unix.cpp
index 4086212..2063408 100644
--- a/src/printsupport/dialogs/qpagesetupdialog_unix.cpp
+++ b/src/printsupport/dialogs/qpagesetupdialog_unix.cpp
@@ -234,6 +234,9 @@ QPageSetupWidget::QPageSetupWidget(QWidget *parent)
       m_printer(0),
       m_outputFormat(QPrinter::PdfFormat),
       m_units(QPageLayout::Point),
+      m_savedUnits(QPageLayout::Point),
+      m_savedPagesPerSheet(-1),
+      m_savedPagesPerSheetLayout(-1),
       m_blockSignals(false),
       m_realCustomPageSizeIndex(-1)
 {
@@ -404,6 +407,7 @@ void QPageSetupWidget::setPrinter(QPrinter *printer, QPrintDevice *printDevice,
     m_printerName = printerName;
     initPageSizes();
     updateWidget();
+    updateSavedValues();
 }
 
 // Update the widget with the current settings
@@ -487,6 +491,7 @@ void QPageSetupWidget::updateWidget()
     m_ui.pageHeight->setEnabled(isCustom);
     m_ui.heightLabel->setEnabled(isCustom);
 
+    m_ui.portrait->setChecked(m_pageLayout.orientation() == QPageLayout::Portrait);
     m_ui.landscape->setChecked(m_pageLayout.orientation() == QPageLayout::Landscape);
 
     m_ui.pagesPerSheetButtonGroup->setEnabled(m_outputFormat == QPrinter::NativeFormat);
@@ -515,6 +520,26 @@ void QPageSetupWidget::setupPrinter() const
 #endif
 }
 
+void QPageSetupWidget::updateSavedValues()
+{
+    m_savedUnits = m_units;
+    m_savedPageLayout = m_pageLayout;
+    m_savedPagesPerSheet = m_ui.pagesPerSheetCombo->currentIndex();
+    m_savedPagesPerSheetLayout = m_ui.pagesPerSheetLayoutCombo->currentIndex();
+}
+
+void QPageSetupWidget::revertToSavedValues()
+{
+    m_units = m_savedUnits;
+    m_pageLayout = m_savedPageLayout;
+    m_pagePreview->setPageLayout(m_pageLayout);
+
+    updateWidget();
+
+    m_ui.pagesPerSheetCombo->setCurrentIndex(m_savedPagesPerSheet);
+    m_ui.pagesPerSheetLayoutCombo->setCurrentIndex(m_savedPagesPerSheetLayout);
+}
+
 // Updates size/preview after the combobox has been changed.
 void QPageSetupWidget::pageSizeChanged()
 {
diff --git a/src/printsupport/dialogs/qpagesetupdialog_unix_p.h b/src/printsupport/dialogs/qpagesetupdialog_unix_p.h
index 0b9723e..82c22c3 100644
--- a/src/printsupport/dialogs/qpagesetupdialog_unix_p.h
+++ b/src/printsupport/dialogs/qpagesetupdialog_unix_p.h
@@ -75,6 +75,8 @@ public:
     void setPrinter(QPrinter *printer, QPrintDevice *printDevice,
                     QPrinter::OutputFormat outputFormat, const QString &printerName);
     void setupPrinter() const;
+    void updateSavedValues();
+    void revertToSavedValues();
 
 private slots:
     void pageSizeChanged();
@@ -100,7 +102,11 @@ private:
     QPrinter::OutputFormat m_outputFormat;
     QString m_printerName;
     QPageLayout m_pageLayout;
+    QPageLayout m_savedPageLayout;
     QPageLayout::Unit m_units;
+    QPageLayout::Unit m_savedUnits;
+    int m_savedPagesPerSheet;
+    int m_savedPagesPerSheetLayout;
     bool m_blockSignals;
     int m_realCustomPageSizeIndex;
 };
diff --git a/src/printsupport/dialogs/qprintdialog_unix.cpp b/src/printsupport/dialogs/qprintdialog_unix.cpp
index a3ba7be..29000bf 100644
--- a/src/printsupport/dialogs/qprintdialog_unix.cpp
+++ b/src/printsupport/dialogs/qprintdialog_unix.cpp
@@ -207,7 +207,6 @@ public:
 private:
     QPrintDialogPrivate *optionsPane;
     bool filePrintersAdded;
-    bool propertiesDialogShown;
 };
 
 class QPrintDialogPrivate : public QAbstractPrintDialogPrivate
@@ -247,11 +246,10 @@ class QOptionTreeItem
 public:
     enum ItemType { Root, Group, Option, Choice };
 
-    QOptionTreeItem(ItemType t, int i, const void *p, const char *desc, QOptionTreeItem *pi)
+    QOptionTreeItem(ItemType t, int i, const void *p, QOptionTreeItem *pi)
         : type(t),
           index(i),
           ptr(p),
-          description(desc),
           parentItem(pi) {}
 
     ~QOptionTreeItem() {
@@ -261,7 +259,6 @@ public:
     ItemType type;
     int index;
     const void *ptr;
-    const char *description;
     QOptionTreeItem *parentItem;
     QList<QOptionTreeItem*> childItems;
 };
@@ -269,14 +266,13 @@ public:
 class QOptionTreeItemOption : public QOptionTreeItem
 {
 public:
-    QOptionTreeItemOption (int i, const void *p, const char *desc, QOptionTreeItem *pi)
-        : QOptionTreeItem(Option, i, p, desc, pi)
+    QOptionTreeItemOption (int i, const void *p, QOptionTreeItem *pi)
+        : QOptionTreeItem(Option, i, p, pi)
     {
     }
 
     int selected;
     int originallySelected;
-    const char *selDescription;
 };
 
 class QPPDOptionsModel : public QAbstractItemModel
@@ -296,6 +292,8 @@ public:
 
     void setCupsOptionsFromItems(QPrinter *printer) const;
     void reject();
+    void updateSavedValues();
+    void revertToSavedValues();
 
     QPrintDevice *currentPrintDevice() const;
     QTextCodec *cupsCodec() const;
@@ -313,6 +311,8 @@ private:
 
     void setCupsOptionsFromItems(QPrinter *printer, QOptionTreeItem *parent) const;
     void reject(QOptionTreeItem *item);
+    void updateSavedValues(QOptionTreeItem *item);
+    void revertToSavedValues(QOptionTreeItem *item);
     void emitDataChanged(QOptionTreeItem *item, const QModelIndex &itemIndex, bool *conflictsFound);
     bool hasConflicts(QOptionTreeItem *item) const;
 
@@ -420,8 +420,14 @@ void QPrintPropertiesDialog::showEvent(QShowEvent *event)
 
 void QPrintPropertiesDialog::reject()
 {
+    widget.pageSetup->revertToSavedValues();
+
+#if QT_CONFIG(cupsjobwidget)
+    m_jobOptions->revertToSavedValues();
+#endif
+
 #if QT_CONFIG(cups)
-    m_cupsOptionsModel->reject();
+    m_cupsOptionsModel->revertToSavedValues();
 #endif
     QDialog::reject();
 }
@@ -437,7 +443,15 @@ void QPrintPropertiesDialog::accept()
         if (answer != QMessageBox::No)
             return;
     }
+    m_cupsOptionsModel->updateSavedValues();
+#endif
+
+#if QT_CONFIG(cupsjobwidget)
+    m_jobOptions->updateSavedValues();
 #endif
+
+    widget.pageSetup->updateSavedValues();
+
     QDialog::accept();
 }
 
@@ -812,9 +826,9 @@ void QPrintDialog::accept()
 */
 QUnixPrintWidgetPrivate::QUnixPrintWidgetPrivate(QUnixPrintWidget *p, QPrinter *prn)
     : parent(p), propertiesDialog(0), printer(prn), optionsPane(0),
-      filePrintersAdded(false), propertiesDialogShown(false)
+      filePrintersAdded(false)
 {
-    q = 0;
+    q = 0;
     if (parent)
         q = qobject_cast<QAbstractPrintDialog*> (parent->parent());
 
@@ -900,7 +914,6 @@ void QUnixPrintWidgetPrivate::_q_printerChanged(int index)
     if (propertiesDialog){
         delete propertiesDialog;
         propertiesDialog = nullptr;
-        propertiesDialogShown = false;
     }
 
     if (filePrintersAdded) {
@@ -993,7 +1006,7 @@ bool QUnixPrintWidgetPrivate::checkFields()
     }
 
 #if QT_CONFIG(cups)
-    if (propertiesDialogShown) {
+    if (propertiesDialog) {
         QCUPSSupport::PagesPerSheet pagesPerSheet = propertiesDialog->widget.pageSetup->m_ui.pagesPerSheetCombo
                                                                     ->currentData().value<QCUPSSupport::PagesPerSheet>();
 
@@ -1030,8 +1043,6 @@ void QUnixPrintWidgetPrivate::setupPrinterProperties()
     }
 
     propertiesDialog = new QPrintPropertiesDialog(q->printer(), &m_currentPrintDevice, outputFormat, printerName, q);
-    propertiesDialog->setResult(QDialog::Rejected);
-    propertiesDialogShown = false;
 }
 
 void QUnixPrintWidgetPrivate::_q_btnPropertiesClicked()
@@ -1039,15 +1050,6 @@ void QUnixPrintWidgetPrivate::_q_btnPropertiesClicked()
     if (!propertiesDialog)
         setupPrinterProperties();
     propertiesDialog->exec();
-    if (!propertiesDialogShown && propertiesDialog->result() == QDialog::Rejected) {
-        // If properties dialog was rejected the dialog is deleted and
-        // the properties are set to defaults when printer is setup
-        delete propertiesDialog;
-        propertiesDialog = nullptr;
-        propertiesDialogShown = false;
-    } else
-        // properties dialog was shown and accepted
-        propertiesDialogShown = true;
 }
 
 void QUnixPrintWidgetPrivate::setupPrinter()
@@ -1072,8 +1074,7 @@ void QUnixPrintWidgetPrivate::setupPrinter()
     if (!propertiesDialog)
         setupPrinterProperties();
 
-    if (propertiesDialog->result() == QDialog::Accepted || !propertiesDialogShown)
-        propertiesDialog->setupPrinter();
+    propertiesDialog->setupPrinter();
 }
 
 /*! \internal
@@ -1153,12 +1154,12 @@ QPPDOptionsModel::QPPDOptionsModel(QPrintDevice *currentPrintDevice, QObject *pa
     , m_cupsCodec(nullptr)
 {
     ppd_file_t *ppd = m_currentPrintDevice->property(PDPK_PpdFile).value<ppd_file_t*>();
-    m_rootItem = new QOptionTreeItem(QOptionTreeItem::Root, 0, ppd, "Root Item", 0);
+    m_rootItem = new QOptionTreeItem(QOptionTreeItem::Root, 0, ppd, nullptr);
 
     if (ppd) {
         m_cupsCodec = QTextCodec::codecForName(ppd->lang_encoding);
         for (int i = 0; i < ppd->num_groups; ++i) {
-            QOptionTreeItem *group = new QOptionTreeItem(QOptionTreeItem::Group, i, &ppd->groups[i], ppd->groups[i].text, m_rootItem);
+            QOptionTreeItem *group = new QOptionTreeItem(QOptionTreeItem::Group, i, &ppd->groups[i], m_rootItem);
             m_rootItem->childItems.append(group);
             parseGroups(group); // parse possible subgroups
             parseOptions(group); // parse options
@@ -1208,11 +1209,18 @@ QVariant QPPDOptionsModel::data(const QModelIndex &index, int role) const
 
     case Qt::DisplayRole: {
         if (index.column() == 0) {
-            return m_cupsCodec->toUnicode(itm->description);
+            if (itm->type == QOptionTreeItem::Option) {
+                const ppd_option_t *option = static_cast<const ppd_option_t*>(itm->ptr);
+                return m_cupsCodec->toUnicode(option->text);
+            } else if (itm->type == QOptionTreeItem::Group) {
+                const ppd_group_t *group = static_cast<const ppd_group_t*>(itm->ptr);
+                return m_cupsCodec->toUnicode(group->text);
+            }
         } else if (itm->type == QOptionTreeItem::Option) {
             QOptionTreeItemOption *itmOption = static_cast<QOptionTreeItemOption *>(itm);
+            const ppd_option_t *option = static_cast<const ppd_option_t*>(itm->ptr);
             if (itmOption->selected > -1)
-                return m_cupsCodec->toUnicode(itmOption->selDescription);
+                return m_cupsCodec->toUnicode(option->choices[itmOption->selected].text);
         }
 
         return QVariant();
@@ -1314,7 +1322,7 @@ void QPPDOptionsModel::parseGroups(QOptionTreeItem *parent)
 
     if (group) {
         for (int i = 0; i < group->num_subgroups; ++i) {
-            QOptionTreeItem *subgroup = new QOptionTreeItem(QOptionTreeItem::Group, i, &group->subgroups[i], group->subgroups[i].text, parent);
+            QOptionTreeItem *subgroup = new QOptionTreeItem(QOptionTreeItem::Group, i, &group->subgroups[i], parent);
             parent->childItems.append(subgroup);
             parseGroups(subgroup); // parse possible subgroups
             parseOptions(subgroup); // parse options
@@ -1346,7 +1354,7 @@ void QPPDOptionsModel::parseOptions(QOptionTreeItem *parent)
     const ppd_group_t *group = static_cast<const ppd_group_t*>(parent->ptr);
     for (int i = 0; i < group->num_options; ++i) {
         if (!isBlacklistedOption(group->options[i].keyword)) {
-            QOptionTreeItemOption *opt = new QOptionTreeItemOption(i, &group->options[i], group->options[i].text, parent);
+            QOptionTreeItemOption *opt = new QOptionTreeItemOption(i, &group->options[i], parent);
             parent->childItems.append(opt);
             parseChoices(opt);
         }
@@ -1358,14 +1366,12 @@ void QPPDOptionsModel::parseChoices(QOptionTreeItemOption *parent)
     const ppd_option_t *option = static_cast<const ppd_option_t*>(parent->ptr);
     bool marked = false;
     for (int i = 0; i < option->num_choices; ++i) {
-        QOptionTreeItem *choice = new QOptionTreeItem(QOptionTreeItem::Choice, i, &option->choices[i], option->choices[i].text, parent);
+        QOptionTreeItem *choice = new QOptionTreeItem(QOptionTreeItem::Choice, i, &option->choices[i], parent);
         if (static_cast<int>(option->choices[i].marked) == 1) {
             parent->selected = i;
-            parent->selDescription = option->choices[i].text;
             marked = true;
         } else if (!marked && qstrcmp(option->choices[i].choice, option->defchoice) == 0) {
             parent->selected = i;
-            parent->selDescription = option->choices[i].text;
         }
         parent->originallySelected = parent->selected;
         parent->childItems.append(choice);
@@ -1436,12 +1442,13 @@ QVariant QPPDOptionsModel::headerData(int section, Qt::Orientation, int role) co
     return QVariant();
 }
 
-void QPPDOptionsModel::reject()
+void QPPDOptionsModel::revertToSavedValues()
 {
-    reject(m_rootItem);
+    revertToSavedValues(m_rootItem);
+    emitConflictsChanged();
 }
 
-void QPPDOptionsModel::reject(QOptionTreeItem *item)
+void QPPDOptionsModel::revertToSavedValues(QOptionTreeItem *item)
 {
     if (item->type == QOptionTreeItem::Option) {
         QOptionTreeItemOption *itemOption = static_cast<QOptionTreeItemOption *>(item);
@@ -1451,10 +1458,27 @@ void QPPDOptionsModel::reject(QOptionTreeItem *item)
                                                                   : option->defchoice;
         const auto values = QStringList{} << QString::fromLatin1(option->keyword) << QString::fromLatin1(choice);
         m_currentPrintDevice->setProperty(PDPK_PpdOption, values);
+        itemOption->selected = itemOption->originallySelected;
     }
 
     for (QOptionTreeItem *child : qAsConst(item->childItems))
-        reject(child);
+        revertToSavedValues(child);
+}
+
+void QPPDOptionsModel::updateSavedValues()
+{
+    updateSavedValues(m_rootItem);
+}
+
+void QPPDOptionsModel::updateSavedValues(QOptionTreeItem *item)
+{
+    if (item->type == QOptionTreeItem::Option) {
+        QOptionTreeItemOption *itemOption = static_cast<QOptionTreeItemOption *>(item);
+        itemOption->originallySelected = itemOption->selected;
+    }
+
+    for (QOptionTreeItem *child : qAsConst(item->childItems))
+        updateSavedValues(child);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1490,8 +1514,10 @@ void QPPDOptionsEditor::setEditorData(QWidget *editor, const QModelIndex &index)
         cb->addItem(QString());
 
     const QPPDOptionsModel *m = static_cast<const QPPDOptionsModel*>(index.model());
-    for (auto *childItem : qAsConst(itm->childItems))
-        cb->addItem(m->cupsCodec()->toUnicode(childItem->description));
+    for (auto *childItem : qAsConst(itm->childItems)) {
+        const ppd_choice_t *choice = static_cast<const ppd_choice_t*>(childItem->ptr);
+        cb->addItem(m->cupsCodec()->toUnicode(choice->text));
+    }
 
     if (itm->selected > -1)
         cb->setCurrentIndex(itm->selected);
@@ -1511,7 +1537,6 @@ void QPPDOptionsEditor::setModelData(QWidget *editor, QAbstractItemModel *model,
     const auto values = QStringList{} << QString::fromLatin1(opt->keyword) << QString::fromLatin1(opt->choices[cb->currentIndex()].choice);
     m->currentPrintDevice()->setProperty(PDPK_PpdOption, values);
     itm->selected = cb->currentIndex();
-    itm->selDescription = static_cast<const ppd_option_t*>(itm->ptr)->choices[itm->selected].text;
 
     m->emitConflictsChanged();
 }
diff --git a/src/printsupport/kernel/qcups_p.h b/src/printsupport/kernel/qcups_p.h
index afddfdb..4b27632 100644
--- a/src/printsupport/kernel/qcups_p.h
+++ b/src/printsupport/kernel/qcups_p.h
@@ -145,13 +145,19 @@ public:
 
     struct JobSheets
     {
-        BannerPage startBannerPage = QCUPSSupport::NoBanner;
-        BannerPage endBannerPage = QCUPSSupport::NoBanner;
+        JobSheets(BannerPage s = NoBanner, BannerPage e = NoBanner)
+         : startBannerPage(s), endBannerPage(e) {}
+
+        BannerPage startBannerPage;
+        BannerPage endBannerPage;
     };
     static JobSheets parseJobSheets(const QString &jobSheets);
 
     struct JobHoldUntilWithTime
     {
+        JobHoldUntilWithTime(JobHoldUntil jh = NoHold, const QTime &t = QTime())
+            : jobHold(jh), time(t) {}
+
         JobHoldUntil jobHold;
         QTime time;
     };
diff --git a/src/printsupport/widgets/qcupsjobwidget.cpp b/src/printsupport/widgets/qcupsjobwidget.cpp
index 7525d7f..dcdb933 100644
--- a/src/printsupport/widgets/qcupsjobwidget.cpp
+++ b/src/printsupport/widgets/qcupsjobwidget.cpp
@@ -77,6 +77,8 @@ QCupsJobWidget::QCupsJobWidget(QPrinter *printer, QPrintDevice *printDevice, QWi
     initJobBilling();
     initJobPriority();
     initBannerPages();
+
+    updateSavedValues();
 }
 
 QCupsJobWidget::~QCupsJobWidget()
@@ -91,6 +93,27 @@ void QCupsJobWidget::setupPrinter()
     QCUPSSupport::setBannerPages(m_printer, startBannerPage(), endBannerPage());
 }
 
+void QCupsJobWidget::updateSavedValues()
+{
+    m_savedJobHoldWithTime = { jobHold(), jobHoldTime() };
+    m_savedJobBilling = jobBilling();
+    m_savedPriority = jobPriority();
+    m_savedJobSheets = { startBannerPage(), endBannerPage() };
+}
+
+void QCupsJobWidget::revertToSavedValues()
+{
+    setJobHold(m_savedJobHoldWithTime.jobHold, m_savedJobHoldWithTime.time);
+    toggleJobHoldTime();
+
+    setJobBilling(m_savedJobBilling);
+
+    setJobPriority(m_savedPriority);
+
+    setStartBannerPage(m_savedJobSheets.startBannerPage);
+    setEndBannerPage(m_savedJobSheets.endBannerPage);
+}
+
 void QCupsJobWidget::initJobHold()
 {
     m_ui.jobHoldComboBox->addItem(tr("Print Immediately"),             QVariant::fromValue(QCUPSSupport::NoHold));
@@ -154,7 +177,7 @@ void QCupsJobWidget::initJobBilling()
 
 void QCupsJobWidget::setJobBilling(const QString &jobBilling)
 {
-    m_ui.jobBillingLineEdit->insert(jobBilling);
+    m_ui.jobBillingLineEdit->setText(jobBilling);
 }
 
 QString QCupsJobWidget::jobBilling() const
diff --git a/src/printsupport/widgets/qcupsjobwidget_p.h b/src/printsupport/widgets/qcupsjobwidget_p.h
index dcec27a..4b6b047 100644
--- a/src/printsupport/widgets/qcupsjobwidget_p.h
+++ b/src/printsupport/widgets/qcupsjobwidget_p.h
@@ -75,6 +75,8 @@ public:
     explicit QCupsJobWidget(QPrinter *printer, QPrintDevice *printDevice, QWidget *parent = nullptr);
     ~QCupsJobWidget();
     void setupPrinter();
+    void updateSavedValues();
+    void revertToSavedValues();
 
 private Q_SLOTS:
     void toggleJobHoldTime();
@@ -106,6 +108,11 @@ private:
     QPrintDevice *m_printDevice;
     Ui::QCupsJobWidget m_ui;
 
+    QCUPSSupport::JobHoldUntilWithTime m_savedJobHoldWithTime;
+    QString m_savedJobBilling;
+    int m_savedPriority;
+    QCUPSSupport::JobSheets m_savedJobSheets;
+
     Q_DISABLE_COPY(QCupsJobWidget)
 };
 
-- 
2.7.4

Places

File 0012-Unix-Fix-usability-of-the-print-properties-dialog.patch of Package libqt5-qtbase (Revision 615ef0f82aaa28ec93c6122d1e3957f9)

Places