From d94ccf310a9ca01593750a34f743ec652f6a344e Mon Sep 17 00:00:00 2001 From: Albert Astals Cid 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 Reviewed-by: Frederik Gladhorn --- 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 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 (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(); @@ -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(); - 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(itm->ptr); + return m_cupsCodec->toUnicode(option->text); + } else if (itm->type == QOptionTreeItem::Group) { + const ppd_group_t *group = static_cast(itm->ptr); + return m_cupsCodec->toUnicode(group->text); + } } else if (itm->type == QOptionTreeItem::Option) { QOptionTreeItemOption *itmOption = static_cast(itm); + const ppd_option_t *option = static_cast(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(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(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(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(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(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(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(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(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