Merge lp:~ahayzen/ubuntu-printing-app/page-range-validate-and-count into lp:ubuntu-printing-app
- page-range-validate-and-count
- Merge into trunk
Status: | Approved |
---|---|
Approved by: | Michael Sheldon |
Approved revision: | 73 |
Proposed branch: | lp:~ahayzen/ubuntu-printing-app/page-range-validate-and-count |
Merge into: | lp:ubuntu-printing-app |
Diff against target: |
918 lines (+666/-36) 11 files modified
po/ubuntu-printing-app.pot (+25/-9) ubuntu-printing-app/backend/CMakeLists.txt (+2/-1) ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp (+2/-0) ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp (+192/-0) ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h (+75/-0) ubuntu-printing-app/components/LabelRow.qml (+2/-0) ubuntu-printing-app/pages/PrintPage.qml (+53/-21) ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml (+8/-0) ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml (+152/-5) ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt (+4/-0) ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp (+151/-0) |
To merge this branch: | bzr merge lp:~ahayzen/ubuntu-printing-app/page-range-validate-and-count |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Sheldon (community) | Approve | ||
Review via email: mp+321547@code.launchpad.net |
Commit message
* Add validation to copies and page range fields with tests
Description of the change
* Add validation to copies and page range fields with tests
Andrew Hayzen (ahayzen) wrote : | # |
As discussed, cups doesn't seem to support overlapping ranges or descending [0]. So for now we are going to do de-duplication and ascending order. It seems that other apps such as documentviewer (gtk), must rendering an intermediate PDF which has the page order how they want.
Andrew Hayzen (ahayzen) wrote : | # |
Reported bug 1680048 for advanced page ranges. And now setting Qt.ImhPreferNumbers for the page range field.
Michael Sheldon (michael-sheldon) wrote : | # |
Looks good :)
Unmerged revisions
- 73. By Andrew Hayzen
-
* Set Qt.ImhPreferNumbers for page-ranges text field
- 72. By Andrew Hayzen
-
* Remove unneeded TODO
- 71. By Andrew Hayzen
-
* Ensure NoError state is always set
* Update naming of error for 5-3 - 70. By Andrew Hayzen
-
* Rebase onto lp:ubuntu-printing-app
- 69. By Andrew Hayzen
-
* Add validation to copies field with tests
- 68. By Andrew Hayzen
-
* Add page range validation and counting with tests
Preview Diff
1 | === modified file 'po/ubuntu-printing-app.pot' |
2 | --- po/ubuntu-printing-app.pot 2017-03-21 17:57:19 +0000 |
3 | +++ po/ubuntu-printing-app.pot 2017-04-05 12:09:01 +0000 |
4 | @@ -8,7 +8,7 @@ |
5 | msgstr "" |
6 | "Project-Id-Version: PACKAGE VERSION\n" |
7 | "Report-Msgid-Bugs-To: \n" |
8 | -"POT-Creation-Date: 2017-03-21 17:56+0000\n" |
9 | +"POT-Creation-Date: 2017-04-05 12:51+0100\n" |
10 | "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" |
11 | "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" |
12 | "Language-Team: LANGUAGE <LL@li.org>\n" |
13 | @@ -185,39 +185,55 @@ |
14 | msgid "Copies" |
15 | msgstr "" |
16 | |
17 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:131 |
18 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:130 |
19 | msgid "All" |
20 | msgstr "" |
21 | |
22 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:131 |
23 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:130 |
24 | msgid "Range" |
25 | msgstr "" |
26 | |
27 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:135 |
28 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:134 |
29 | msgid "Pages" |
30 | msgstr "" |
31 | |
32 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:177 |
33 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:175 |
34 | msgid "eg 1-3,8" |
35 | msgstr "" |
36 | |
37 | #: ../ubuntu-printing-app/pages/PrintPage.qml:186 |
38 | +msgid "No number given in range" |
39 | +msgstr "" |
40 | + |
41 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:189 |
42 | +msgid "Page is less than one" |
43 | +msgstr "" |
44 | + |
45 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:192 |
46 | +msgid "Page is higher than document page count" |
47 | +msgstr "" |
48 | + |
49 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:195 |
50 | +msgid "Second page cannot be lower than first" |
51 | +msgstr "" |
52 | + |
53 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:211 |
54 | msgid "Two-sided" |
55 | msgstr "" |
56 | |
57 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:207 |
58 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:232 |
59 | msgid "Color" |
60 | msgstr "" |
61 | |
62 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:228 |
63 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:253 |
64 | msgid "Quality" |
65 | msgstr "" |
66 | |
67 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:246 |
68 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:271 |
69 | msgid "Collate" |
70 | msgstr "" |
71 | |
72 | -#: ../ubuntu-printing-app/pages/PrintPage.qml:266 |
73 | +#: ../ubuntu-printing-app/pages/PrintPage.qml:291 |
74 | msgid "Reverse" |
75 | msgstr "" |
76 | |
77 | |
78 | === modified file 'ubuntu-printing-app/backend/CMakeLists.txt' |
79 | --- ubuntu-printing-app/backend/CMakeLists.txt 2017-03-07 15:01:56 +0000 |
80 | +++ ubuntu-printing-app/backend/CMakeLists.txt 2017-04-05 12:09:01 +0000 |
81 | @@ -9,8 +9,9 @@ |
82 | UbuntuPrintingAppSRC |
83 | UbuntuPrintingApp/backend.cpp |
84 | UbuntuPrintingApp/document.cpp |
85 | + UbuntuPrintingApp/pagehelper.cpp |
86 | + UbuntuPrintingApp/pagerangevalidator.cpp |
87 | UbuntuPrintingApp/popplerimageprovider.cpp |
88 | - UbuntuPrintingApp/pagehelper.cpp |
89 | ) |
90 | |
91 | add_library(${LIBNAME} SHARED ${UbuntuPrintingAppSRC}) |
92 | |
93 | === modified file 'ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp' |
94 | --- ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp 2017-01-30 15:19:57 +0000 |
95 | +++ ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp 2017-04-05 12:09:01 +0000 |
96 | @@ -23,6 +23,7 @@ |
97 | |
98 | #include "document.h" |
99 | #include "pagehelper.h" |
100 | +#include "pagerangevalidator.h" |
101 | #include "popplerimageprovider.h" |
102 | |
103 | void BackendPlugin::registerTypes(const char *uri) |
104 | @@ -31,6 +32,7 @@ |
105 | |
106 | qmlRegisterType<Document>(uri, 1, 0, "Document"); |
107 | qmlRegisterType<PageHelper>(uri, 1, 0, "PageHelper"); |
108 | + qmlRegisterType<PageRangeValidator>(uri, 1, 0, "PageRangeValidator"); |
109 | } |
110 | |
111 | void BackendPlugin::initializeEngine(QQmlEngine *engine, const char *uri) |
112 | |
113 | === added file 'ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp' |
114 | --- ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp 1970-01-01 00:00:00 +0000 |
115 | +++ ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp 2017-04-05 12:09:01 +0000 |
116 | @@ -0,0 +1,192 @@ |
117 | +/* |
118 | + * Copyright 2017 Canonical Ltd. |
119 | + * |
120 | + * This file is part of ubuntu-printing-app. |
121 | + * |
122 | + * ubuntu-printing-app is free software; you can redistribute it and/or modify |
123 | + * it under the terms of the GNU General Public License as published by |
124 | + * the Free Software Foundation; version 3. |
125 | + * |
126 | + * ubuntu-printing-app is distributed in the hope that it will be useful, |
127 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
128 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
129 | + * GNU General Public License for more details. |
130 | + * |
131 | + * You should have received a copy of the GNU General Public License |
132 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
133 | + * |
134 | + * Authored-by: Andrew Hayzen <andrew.hayzen@canonical.com> |
135 | + */ |
136 | + |
137 | +#include "pagerangevalidator.h" |
138 | + |
139 | +#include <QtCore/QRegularExpressionMatch> |
140 | +#include <QtCore/QList> |
141 | + |
142 | +PageRangeValidator::PageRangeValidator(QObject *parent) |
143 | + : QValidator(parent) |
144 | + , m_count(0) |
145 | + , m_document_count(0) |
146 | + , m_error(PageRangeError::NoError) |
147 | + , m_pages({}) |
148 | + , m_validate_re(QStringLiteral("^(\\d+)?[-]?(\\d+)?([,](\\d+)?[-]?(\\d+)?)*$")) |
149 | + , m_validate_values_re(QStringLiteral("^(\\d*)?[-]?(\\d*)?$")) |
150 | +{ |
151 | + |
152 | +} |
153 | + |
154 | +int PageRangeValidator::count() const |
155 | +{ |
156 | + return m_count; |
157 | +} |
158 | + |
159 | +int PageRangeValidator::documentCount() const |
160 | +{ |
161 | + return m_document_count; |
162 | +} |
163 | + |
164 | +PageRangeValidator::PageRangeError PageRangeValidator::error() const |
165 | +{ |
166 | + return m_error; |
167 | +} |
168 | + |
169 | +QString PageRangeValidator::pages() const |
170 | +{ |
171 | + // Convert the pages to a list of comma's for QML and cups |
172 | + QStringList list; |
173 | + |
174 | + // Get a list and sort while still int so that 10 > 2 |
175 | + QList<int> pages = m_pages.toList(); |
176 | + qSort(pages); |
177 | + |
178 | + Q_FOREACH(int page, pages) { |
179 | + list << QString::number(page); |
180 | + } |
181 | + |
182 | + return list.join(","); |
183 | +} |
184 | + |
185 | +void PageRangeValidator::setCount(const int count) |
186 | +{ |
187 | + if (m_count != count) { |
188 | + m_count = count; |
189 | + |
190 | + Q_EMIT countChanged(m_count); |
191 | + } |
192 | +} |
193 | + |
194 | +void PageRangeValidator::setDocumentCount(const int documentCount) |
195 | +{ |
196 | + if (m_document_count != documentCount) { |
197 | + m_document_count = documentCount; |
198 | + |
199 | + Q_EMIT documentCountChanged(m_document_count); |
200 | + } |
201 | +} |
202 | + |
203 | +void PageRangeValidator::setError(const PageRangeValidator::PageRangeError error) |
204 | +{ |
205 | + if (m_error != error) { |
206 | + m_error = error; |
207 | + |
208 | + Q_EMIT errorChanged(m_error); |
209 | + } |
210 | +} |
211 | + |
212 | +void PageRangeValidator::setPages(const QSet<int> pages) |
213 | +{ |
214 | + if (m_pages != pages) { |
215 | + m_pages = pages; |
216 | + |
217 | + setCount(m_pages.count()); |
218 | + |
219 | + Q_EMIT pagesChanged(this->pages()); |
220 | + } |
221 | +} |
222 | + |
223 | +QValidator::State PageRangeValidator::validate(QString &input, int &pos) const |
224 | +{ |
225 | + Q_UNUSED(pos); |
226 | + |
227 | + // TODO: maybe use partial match for 1-3-5? |
228 | + // QRegularExpression::PartialPreferCompleteMatch |
229 | + // QValidator::State::Intermediate |
230 | + // however then it allows 1,------ |
231 | + QRegularExpressionMatch match = m_validate_re.match(input, 0); |
232 | + |
233 | + if (match.hasMatch()) { |
234 | + return QValidator::State::Acceptable; |
235 | + } else { |
236 | + return QValidator::State::Invalid; |
237 | + } |
238 | +} |
239 | + |
240 | +bool PageRangeValidator::validateValues(QString input) |
241 | +{ |
242 | + bool valid = true; |
243 | + |
244 | + // If the given value is empty then set the whole range |
245 | + if (input.isEmpty()) { |
246 | + setPages({}); |
247 | + setCount(documentCount()); |
248 | + |
249 | + return valid; |
250 | + } |
251 | + |
252 | + QSet<int> pages; |
253 | + setError(PageRangeError::NoError); |
254 | + |
255 | + Q_FOREACH(QString part, input.split(",")) { |
256 | + QRegularExpressionMatch matches = m_validate_values_re.match(part); |
257 | + |
258 | + QString low = matches.captured(1); |
259 | + QString high = matches.captured(2); |
260 | + |
261 | + if (low.isEmpty() && high.isEmpty()) { |
262 | + setError(PageRangeError::NoPageInRange); |
263 | + valid = false; |
264 | + break; |
265 | + } else if (high.isEmpty()) { |
266 | + if (part.contains("-")) { |
267 | + // 1- add the range from the lower until the document count |
268 | + high = QString::number(documentCount()); |
269 | + } else { |
270 | + // 1 Add the given page |
271 | + high = low; |
272 | + } |
273 | + } else if (low.isEmpty()) { |
274 | + // -3 add the range up to the higher number |
275 | + low = QString::number(1); |
276 | + } else { |
277 | + // high and low set, we don't need todo anything |
278 | + } |
279 | + |
280 | + // Convert to ints |
281 | + int lower = low.toInt(); |
282 | + int higher = high.toInt(); |
283 | + |
284 | + // Validate the numbers |
285 | + if (lower < 1 || higher < 1) { |
286 | + setError(PageRangeError::PageLessThanOne); |
287 | + valid = false; |
288 | + break; |
289 | + } else if (lower > documentCount() || higher > documentCount()) { |
290 | + setError(PageRangeError::PageHigherThanDocument); |
291 | + valid = false; |
292 | + break; |
293 | + } else if (lower > higher) { |
294 | + setError(PageRangeError::PageLowGreaterThanHigh); |
295 | + valid = false; |
296 | + break; |
297 | + } else { |
298 | + // Valid so lets push given pages numbers |
299 | + for (int i=lower; i <= higher; i++) { |
300 | + pages.insert(i); |
301 | + } |
302 | + } |
303 | + } |
304 | + |
305 | + setPages(pages); |
306 | + |
307 | + return valid; |
308 | +} |
309 | |
310 | === added file 'ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h' |
311 | --- ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h 1970-01-01 00:00:00 +0000 |
312 | +++ ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h 2017-04-05 12:09:01 +0000 |
313 | @@ -0,0 +1,75 @@ |
314 | +/* |
315 | + * Copyright 2017 Canonical Ltd. |
316 | + * |
317 | + * This file is part of ubuntu-printing-app. |
318 | + * |
319 | + * ubuntu-printing-app is free software; you can redistribute it and/or modify |
320 | + * it under the terms of the GNU General Public License as published by |
321 | + * the Free Software Foundation; version 3. |
322 | + * |
323 | + * ubuntu-printing-app is distributed in the hope that it will be useful, |
324 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
325 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
326 | + * GNU General Public License for more details. |
327 | + * |
328 | + * You should have received a copy of the GNU General Public License |
329 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
330 | + * |
331 | + * Authored-by: Andrew Hayzen <andrew.hayzen@canonical.com> |
332 | + */ |
333 | +#ifndef PAGERANGEVALIDATOR_H |
334 | +#define PAGERANGEVALIDATOR_H |
335 | + |
336 | +#include <QtCore/QRegularExpression> |
337 | +#include <QtCore/QSet> |
338 | +#include <QtGui/QValidator> |
339 | + |
340 | +class PageRangeValidator : public QValidator |
341 | +{ |
342 | + Q_OBJECT |
343 | + |
344 | + Q_PROPERTY(int count READ count NOTIFY countChanged) |
345 | + Q_PROPERTY(int documentCount READ documentCount WRITE setDocumentCount NOTIFY documentCountChanged) |
346 | + Q_PROPERTY(PageRangeError error READ error NOTIFY errorChanged) |
347 | + Q_PROPERTY(QString pages READ pages NOTIFY pagesChanged) |
348 | +public: |
349 | + PageRangeValidator(QObject *parent=Q_NULLPTR); |
350 | + |
351 | + enum PageRangeError { |
352 | + NoError, |
353 | + NoPageInRange, |
354 | + PageLessThanOne, |
355 | + PageHigherThanDocument, |
356 | + PageLowGreaterThanHigh, |
357 | + }; |
358 | + Q_ENUM(PageRangeError) |
359 | + |
360 | + int count() const; |
361 | + int documentCount() const; |
362 | + PageRangeError error() const; |
363 | + QString pages() const; |
364 | + virtual QValidator::State validate(QString &input, int &pos) const override; |
365 | + |
366 | + Q_INVOKABLE bool validateValues(QString input); |
367 | +Q_SIGNALS: |
368 | + void countChanged(int count); |
369 | + void documentCountChanged(int documentCount); |
370 | + void errorChanged(PageRangeError error); |
371 | + void pagesChanged(QString pages); |
372 | +public Q_SLOTS: |
373 | + void setDocumentCount(const int documentCount); |
374 | +private Q_SLOTS: |
375 | + void setCount(const int count); |
376 | + void setPages(const QSet<int> pages); |
377 | + void setError(const PageRangeError error); |
378 | +private: |
379 | + int m_count; |
380 | + int m_document_count; |
381 | + PageRangeError m_error; |
382 | + QSet<int> m_pages; |
383 | + QRegularExpression m_validate_re; |
384 | + QRegularExpression m_validate_values_re; |
385 | +}; |
386 | + |
387 | + |
388 | +#endif // PAGERANGEVALIDATOR_H |
389 | |
390 | === modified file 'ubuntu-printing-app/components/LabelRow.qml' |
391 | --- ubuntu-printing-app/components/LabelRow.qml 2017-03-07 13:46:02 +0000 |
392 | +++ ubuntu-printing-app/components/LabelRow.qml 2017-04-05 12:09:01 +0000 |
393 | @@ -26,6 +26,7 @@ |
394 | property alias enabled: secondaryLabel.enabled |
395 | property alias primaryText: primaryLabel.text |
396 | property alias secondaryText: secondaryLabel.text |
397 | + property alias secondaryColor: secondaryLabel.color |
398 | |
399 | Label { |
400 | id: primaryLabel |
401 | @@ -40,5 +41,6 @@ |
402 | Layout.preferredWidth: units.gu(10) |
403 | objectName: "secondary" |
404 | verticalAlignment: Text.AlignVCenter |
405 | + wrapMode: Text.WrapAtWordBoundaryOrAnywhere |
406 | } |
407 | } |
408 | |
409 | === modified file 'ubuntu-printing-app/pages/PrintPage.qml' |
410 | --- ubuntu-printing-app/pages/PrintPage.qml 2017-03-21 18:00:19 +0000 |
411 | +++ ubuntu-printing-app/pages/PrintPage.qml 2017-04-05 12:09:01 +0000 |
412 | @@ -105,9 +105,8 @@ |
413 | inputMethodHints: Qt.ImhDigitsOnly |
414 | objectName: "copiesTextField" |
415 | text: i18n.tr("Copies") |
416 | - validator: IntValidator { |
417 | - bottom: 1 |
418 | - top: 999 |
419 | + validator: RegExpValidator { |
420 | + regExp: /^[1-9](\d)*$/ |
421 | } |
422 | value: printing.printerJob.copies |
423 | |
424 | @@ -151,23 +150,23 @@ |
425 | TextFieldRow { |
426 | id: pageRangeTextField |
427 | enabled: printing.isLoaded && !printing.pdfMode |
428 | + inputMethodHints: Qt.ImhPreferNumbers |
429 | objectName: "pageRangeTextField" |
430 | - validator: RegExpValidator { |
431 | -// regExp: "" // TODO: validate to only 0-9||9-0||0 , |
432 | + validator: PageRangeValidator { |
433 | + documentCount: document.count |
434 | + |
435 | + // validate() is const, so bind a secondary validateValues |
436 | + // which checks the values in the range are valid |
437 | + // and sets the count/pages |
438 | + readonly property bool validValues: validateValues(pageRangeTextField.value) |
439 | } |
440 | visible: pageRangeSelector.selectedValue === PrinterEnum.PageRange |
441 | |
442 | - onValueChanged: { |
443 | - if (printing.printerJob.printRange !== value) { |
444 | - printing.printerJob.printRange = value |
445 | - } |
446 | - } |
447 | - |
448 | Binding { |
449 | - target: pageRangeTextField |
450 | - property: "value" |
451 | - when: printing.printerJob && pageRangeTextField.enabled |
452 | - value: printing.printerJob.printRange |
453 | + target: printing.printerJob |
454 | + property: "printRange" |
455 | + when: printing.printerJob && pageRangeTextField.enabled && pageRangeTextField.validator.validValues |
456 | + value: pageRangeTextField.validator.pages |
457 | } |
458 | } |
459 | |
460 | @@ -178,6 +177,33 @@ |
461 | visible: pageRangeSelector.selectedValue === PrinterEnum.PageRange |
462 | } |
463 | |
464 | + LabelRow { |
465 | + enabled: printing.isLoaded && !printing.pdfMode |
466 | + objectName: "pageRangeErrorStrLabel" |
467 | + secondaryColor: theme.palette.normal.negative |
468 | + secondaryText: { |
469 | + switch (pageRangeTextField.validator.error) { |
470 | + case PageRangeValidator.NoPageInRange: |
471 | + i18n.tr("No number given in range") |
472 | + break; |
473 | + case PageRangeValidator.PageLessThanOne: |
474 | + i18n.tr("Page is less than one") |
475 | + break; |
476 | + case PageRangeValidator.PageHigherThanDocument: |
477 | + i18n.tr("Page is higher than document page count") |
478 | + break; |
479 | + case PageRangeValidator.PageLowGreaterThanHigh: |
480 | + i18n.tr("Second page cannot be lower than first") |
481 | + break; |
482 | + default: |
483 | + case PageRangeValidator.NoError: |
484 | + "" |
485 | + break; |
486 | + } |
487 | + } |
488 | + visible: pageRangeSelector.selectedValue === PrinterEnum.PageRange && !pageRangeTextField.validator.validValues |
489 | + } |
490 | + |
491 | SelectorRow { |
492 | id: duplexSelector |
493 | enabled: printing.isEditable && currentDocument.count > 1 && printing.printer.supportedDuplexModes.length > 1 |
494 | @@ -297,16 +323,22 @@ |
495 | right: parent.right |
496 | rightMargin: units.gu(1) |
497 | } |
498 | - canPrint: printing.isLoaded |
499 | + canPrint: { |
500 | + if (printing.pdfMode) { |
501 | + printing.isLoaded |
502 | + } else if (printing.printerJob.printRangeMode === PrinterEnum.PageRange) { |
503 | + // If the page range is set, then check it is valid |
504 | + printing.isLoaded && copiesSelector.acceptableInput && pageRangeTextField.validator.validValues |
505 | + } else { |
506 | + printing.isLoaded && copiesSelector.acceptableInput |
507 | + } |
508 | + } |
509 | objectName: "printRow" |
510 | pdfMode: printing.pdfMode |
511 | - // TODO: This should count the range not all pages |
512 | - // roundUp((pageCount * copies) / duplex) |
513 | sheets: { |
514 | - // If range is selected and a value exists, then set the sheets to zero |
515 | - // for now. Which results in not showing the number sheets |
516 | + // If the page range is set then use it |
517 | if (printing.printerJob.printRangeMode === PrinterEnum.PageRange && printing.printerJob.printRange.length > 0) { |
518 | - 0 |
519 | + Math.ceil((pageRangeTextField.validator.count * printing.printerJob.copies) / (printing.printerJob.isTwoSided ? 2 : 1)) |
520 | } else { |
521 | Math.ceil((document.count * printing.printerJob.copies) / (printing.printerJob.isTwoSided ? 2 : 1)) |
522 | } |
523 | |
524 | === modified file 'ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml' |
525 | --- ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml 2017-03-17 12:34:49 +0000 |
526 | +++ ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml 2017-04-05 12:09:01 +0000 |
527 | @@ -36,11 +36,13 @@ |
528 | |
529 | readonly property bool dataEnabled: true |
530 | readonly property string dataPrimaryText: "Primary" |
531 | + readonly property string dataSecondaryColor: "#ff0000" |
532 | readonly property string dataSecondaryText: "Secondary" |
533 | |
534 | function init() { |
535 | labelRow.enabled = dataEnabled; |
536 | labelRow.primaryText = dataPrimaryText; |
537 | + labelRow.secondaryColor = dataSecondaryColor; |
538 | labelRow.secondaryText = dataSecondaryText; |
539 | |
540 | waitForRendering(labelRow); |
541 | @@ -69,5 +71,11 @@ |
542 | var secondary = findChild(labelRow, "secondary"); |
543 | compare(secondary.text, dataSecondaryText); |
544 | } |
545 | + |
546 | + function test_secondaryColor() { |
547 | + // Check secondary color is correct |
548 | + var secondary = findChild(labelRow, "secondary"); |
549 | + compare(secondary.color, dataSecondaryColor); |
550 | + } |
551 | } |
552 | } |
553 | |
554 | === modified file 'ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml' |
555 | --- ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml 2017-03-21 17:57:19 +0000 |
556 | +++ ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml 2017-04-05 12:09:01 +0000 |
557 | @@ -119,11 +119,15 @@ |
558 | |
559 | mockPrinting.printerJob.collate = true; |
560 | mockPrinting.printerJob.copies = 1; |
561 | + mockPrinting.printerJob.isTwoSided = false; |
562 | mockPrinting.printerJob.printRangeMode = PrinterEnum.AllPages; |
563 | mockPrinting.printerJob.reverse = false; |
564 | |
565 | mockPrinting.printerSelectedIndex = 0; |
566 | |
567 | + var printRange = findChild(printPage, "pageRangeTextField"); |
568 | + printRange.value = ""; |
569 | + |
570 | cancelSpy.clear(); |
571 | confirmSpy.clear(); |
572 | |
573 | @@ -261,6 +265,24 @@ |
574 | compare(mockPrinting.printerJob.copies, 2); |
575 | } |
576 | |
577 | + function test_copiesKeyClickZero() { |
578 | + // Check copies starting value is correct |
579 | + var copies = findChild(printPage, "copiesTextField"); |
580 | + compare(copies.value, "1"); |
581 | + |
582 | + // Click on the textField |
583 | + mouseClick(copies); |
584 | + |
585 | + // Clear the current text and try to enter "01" |
586 | + keyClick(Qt.Key_Backspace); |
587 | + keyClick(Qt.Key_0); |
588 | + keyClick(Qt.Key_1); |
589 | + |
590 | + // Check that "1" is set to the backend |
591 | + tryCompare(copies, "value", "1", timeout, "Copies value did not change"); |
592 | + compare(mockPrinting.printerJob.copies, 1); |
593 | + } |
594 | + |
595 | function test_duplex() { |
596 | var duplex = findChild(printPage, "duplexSelector"); |
597 | |
598 | @@ -406,6 +428,73 @@ |
599 | } |
600 | } |
601 | |
602 | + function test_printRange() { |
603 | + document.url = Qt.resolvedUrl("../resources/pdf/mixed_portrait.pdf"); |
604 | + |
605 | + // Change to PageRange mode |
606 | + mockPrinting.printerJob.printRangeMode = PrinterEnum.PageRange; |
607 | + |
608 | + var printRange = findChild(printPage, "pageRangeTextField"); |
609 | + var printRangeError = findChild(printPage, "pageRangeErrorStrLabel"); |
610 | + var printRow = findChild(printPage, "printRow"); |
611 | + |
612 | + // Test that a range sets printRange in printerJob |
613 | + |
614 | + // Click on the textField |
615 | + mouseClick(printRange); |
616 | + |
617 | + // Clear the current text and enter "2" |
618 | + keyClick(Qt.Key_Backspace); |
619 | + keyClick(Qt.Key_2); |
620 | + |
621 | + compare(mockPrinting.printerJob.printRange, "2"); |
622 | + compare(printRangeError.visible, false); |
623 | + compare(printRangeError.secondaryText, ""); |
624 | + compare(printRow.canPrint, true); |
625 | + |
626 | + |
627 | + // Test that a invalid range shows the error and disables button |
628 | + |
629 | + // Click on the textField |
630 | + mouseClick(printRange); |
631 | + |
632 | + // Clear the current text and enter "," |
633 | + keyClick(Qt.Key_Backspace); |
634 | + keyClick(Qt.Key_Comma); |
635 | + |
636 | + compare(printRangeError.visible, true); |
637 | + compare(printRangeError.secondaryText, i18n.tr("No number given in range")); |
638 | + compare(printRow.canPrint, false); |
639 | + |
640 | + // Clear the current text and enter "0" |
641 | + keyClick(Qt.Key_Backspace); |
642 | + keyClick(Qt.Key_0); |
643 | + |
644 | + compare(printRangeError.visible, true); |
645 | + compare(printRangeError.secondaryText, i18n.tr("Page is less than one")); |
646 | + compare(printRow.canPrint, false); |
647 | + |
648 | + // Clear the current text and enter "10" |
649 | + keyClick(Qt.Key_Backspace); |
650 | + keyClick(Qt.Key_1); |
651 | + keyClick(Qt.Key_0); |
652 | + |
653 | + compare(printRangeError.visible, true); |
654 | + compare(printRangeError.secondaryText, i18n.tr("Page is higher than document page count")); |
655 | + compare(printRow.canPrint, false); |
656 | + |
657 | + // Clear the current text and enter "2-1" |
658 | + keyClick(Qt.Key_Backspace); |
659 | + keyClick(Qt.Key_Backspace); |
660 | + keyClick(Qt.Key_2); |
661 | + keyClick(Qt.Key_Minus); |
662 | + keyClick(Qt.Key_1); |
663 | + |
664 | + compare(printRangeError.visible, true); |
665 | + compare(printRangeError.secondaryText, i18n.tr("Page low is greater than high")); |
666 | + compare(printRow.canPrint, false); |
667 | + } |
668 | + |
669 | function test_printRangeMode() { |
670 | var printRangeMode = findChild(printPage, "pageRangeSelector"); |
671 | var printRange = findChild(printPage, "pageRangeTextField"); |
672 | @@ -503,12 +592,70 @@ |
673 | mockPrinting.printerJob.isTwoSided = true; |
674 | |
675 | compare(printRow.sheets, 2); |
676 | - |
677 | - // Enable page ranges, this results in zero sheets |
678 | - mockPrinting.printerJob.printRange = "-3,5-7,9-"; |
679 | + } |
680 | + |
681 | + function test_sheets_range() { |
682 | + var printRow = findChild(printPage, "printRow"); |
683 | + var printRange = findChild(printPage, "pageRangeTextField"); |
684 | + |
685 | + // has 3 pages |
686 | + document.url = Qt.resolvedUrl("../resources/pdf/mixed_portrait.pdf"); |
687 | + |
688 | + compare(document.count, 3); |
689 | + compare(mockPrinting.printerJob.copies, 1); |
690 | + compare(mockPrinting.printerJob.isTwoSided, false); |
691 | + compare(printRow.sheets, 3); |
692 | + |
693 | + // Set range to 1 check than sheets is correct |
694 | mockPrinting.printerJob.printRangeMode = PrinterEnum.PageRange; |
695 | - |
696 | - compare(printRow.sheets, 0); |
697 | + printRange.value = "1"; |
698 | + |
699 | + compare(printRow.sheets, 1); |
700 | + |
701 | + // Enable twoSided, we should get 0.5 rounded to 1 sheet |
702 | + mockPrinting.printerJob.isTwoSided = true; |
703 | + |
704 | + compare(mockPrinting.printerJob.isTwoSided, true); |
705 | + compare(printRow.sheets, 1); |
706 | + |
707 | + // Enable two copies, we should get 2 copies with duplex, so 1 sheet |
708 | + mockPrinting.printerJob.copies = 2; |
709 | + |
710 | + compare(mockPrinting.printerJob.copies, 2); |
711 | + compare(printRow.sheets, 1); |
712 | + |
713 | + // Disable two sided, so 2 copies and 2 sheets |
714 | + mockPrinting.printerJob.isTwoSided = false; |
715 | + |
716 | + compare(mockPrinting.printerJob.isTwoSided, false); |
717 | + compare(printRow.sheets, 2); |
718 | + |
719 | + |
720 | + // Set the range to 2 sheets |
721 | + printRange.value = "1,2"; |
722 | + |
723 | + mockPrinting.printerJob.copies = 1; |
724 | + mockPrinting.printerJob.isTwoSided = false; |
725 | + |
726 | + compare(printRow.sheets, 2); |
727 | + |
728 | + // Enable twoSided, we should get 2 pages on 1 sheet |
729 | + mockPrinting.printerJob.isTwoSided = true; |
730 | + |
731 | + compare(mockPrinting.printerJob.isTwoSided, true); |
732 | + compare(printRow.sheets, 1); |
733 | + |
734 | + // Enable two copies, we should get 2 copies with duplex, so 2 sheet |
735 | + mockPrinting.printerJob.copies = 2; |
736 | + |
737 | + compare(mockPrinting.printerJob.copies, 2); |
738 | + compare(printRow.sheets, 2); |
739 | + |
740 | + // Disable two sided, so 2 copies and 4 sheets |
741 | + mockPrinting.printerJob.isTwoSided = false; |
742 | + |
743 | + compare(mockPrinting.printerJob.isTwoSided, false); |
744 | + compare(printRow.sheets, 4); |
745 | } |
746 | } |
747 | } |
748 | |
749 | === modified file 'ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt' |
750 | --- ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt 2017-03-15 11:57:40 +0000 |
751 | +++ ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt 2017-04-05 12:09:01 +0000 |
752 | @@ -14,6 +14,10 @@ |
753 | target_link_libraries(testPageHelper UbuntuPrintingAppbackend Qt5::Test Qt5::Gui) |
754 | add_test(tst_pagehelper testPageHelper) |
755 | |
756 | +add_executable(testPageRangeValidator tst_pagerangevalidator.cpp ${PDF_TEST_FILES}) |
757 | +target_link_libraries(testPageRangeValidator UbuntuPrintingAppbackend Qt5::Test Qt5::Gui) |
758 | +add_test(tst_pagerangevalidator testPageRangeValidator) |
759 | + |
760 | add_executable(testPopplerImageProvider tst_popplerimageprovider.cpp ${PDF_TEST_FILES}) |
761 | target_link_libraries(testPopplerImageProvider UbuntuPrintingAppbackend Qt5::Test Qt5::Gui) |
762 | add_test(tst_popplerimageprovider testPopplerImageProvider) |
763 | |
764 | === added file 'ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp' |
765 | --- ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp 1970-01-01 00:00:00 +0000 |
766 | +++ ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp 2017-04-05 12:09:01 +0000 |
767 | @@ -0,0 +1,151 @@ |
768 | +/* |
769 | + * Copyright 2017 Canonical Ltd. |
770 | + * |
771 | + * This file is part of ubuntu-printing-app. |
772 | + * |
773 | + * ubuntu-printing-app is free software; you can redistribute it and/or modify |
774 | + * it under the terms of the GNU General Public License as published by |
775 | + * the Free Software Foundation; version 3. |
776 | + * |
777 | + * ubuntu-printing-app is distributed in the hope that it will be useful, |
778 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
779 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
780 | + * GNU General Public License for more details. |
781 | + * |
782 | + * You should have received a copy of the GNU General Public License |
783 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
784 | + * |
785 | + * Authored-by: Andrew Hayzen <andrew.hayzen@canonical.com> |
786 | + */ |
787 | + |
788 | +#include <QObject> |
789 | +#include <QSignalSpy> |
790 | +#include <QTest> |
791 | + |
792 | +#include "UbuntuPrintingApp/pagerangevalidator.h" |
793 | + |
794 | +class TestPageRangeValidator : public QObject |
795 | +{ |
796 | + Q_OBJECT |
797 | +private Q_SLOTS: |
798 | + void init() |
799 | + { |
800 | + m_validator = new PageRangeValidator(); |
801 | + } |
802 | + void cleanup() |
803 | + { |
804 | + QSignalSpy destroyedSpy(m_validator, SIGNAL(destroyed(QObject*))); |
805 | + m_validator->deleteLater(); |
806 | + QTRY_COMPARE(destroyedSpy.count(), 1); |
807 | + } |
808 | + |
809 | + // Check that the initial values are correct |
810 | + void testInit() |
811 | + { |
812 | + QCOMPARE(m_validator->count(), 0); |
813 | + QCOMPARE(m_validator->documentCount(), 0); |
814 | + QCOMPARE(m_validator->error(), PageRangeValidator::PageRangeError::NoError); |
815 | + QCOMPARE(m_validator->pages(), QStringLiteral("")); |
816 | + } |
817 | + |
818 | + // Check that the document count can be set |
819 | + void testDocumentCount() |
820 | + { |
821 | + QSignalSpy documentCountSpy(m_validator, SIGNAL(documentCountChanged(int))); |
822 | + m_validator->setDocumentCount(10); |
823 | + |
824 | + QTRY_COMPARE(documentCountSpy.count(), 1); |
825 | + QCOMPARE(m_validator->documentCount(), 10); |
826 | + } |
827 | + |
828 | + void testValidate_data() |
829 | + { |
830 | + QTest::addColumn<QString>("input"); |
831 | + QTest::addColumn<bool>("result"); |
832 | + |
833 | + // Good values |
834 | + QTest::newRow("empty") << "" << true; |
835 | + QTest::newRow("single value") << "2" << true; |
836 | + QTest::newRow("multiple values") << "2,3,4" << true; |
837 | + QTest::newRow("no lower") << "-2" << true; |
838 | + QTest::newRow("no upper") << "2-" << true; |
839 | + QTest::newRow("single range") << "2-3" << true; |
840 | + QTest::newRow("range and value") << "2-3,6" << true; |
841 | + QTest::newRow("multiple ranges") << "2-3,6-7" << true; |
842 | + QTest::newRow("overlapping ranges") << "2-8,6-10" << true; |
843 | + QTest::newRow("dash") << "-" << true; |
844 | + QTest::newRow("comma") << "," << true; |
845 | + QTest::newRow("double comma") << ",," << true; |
846 | + |
847 | + // Bad values |
848 | + QTest::newRow("no comma multi range") << "1-2-3" << false; |
849 | + QTest::newRow("double dash") << "--" << false; |
850 | + QTest::newRow("double dash range") << "2--3" << false; |
851 | + QTest::newRow("letter") << "a" << false; |
852 | + } |
853 | + |
854 | + void testValidate() |
855 | + { |
856 | + QFETCH(QString, input); |
857 | + QFETCH(bool, result); |
858 | + |
859 | + int pos = 0; |
860 | + |
861 | + QCOMPARE(m_validator->validate(input, pos), result ? QValidator::State::Acceptable : QValidator::State::Invalid); |
862 | + } |
863 | + |
864 | + void testValidateValues_data() |
865 | + { |
866 | + QTest::addColumn<QString>("input"); |
867 | + QTest::addColumn<bool>("result"); |
868 | + QTest::addColumn<int>("count"); |
869 | + QTest::addColumn<QString>("pages"); |
870 | + QTest::addColumn<PageRangeValidator::PageRangeError>("error"); |
871 | + |
872 | + // Good values |
873 | + QTest::newRow("empty") << "" << true << 10 << "" << PageRangeValidator::NoError; |
874 | + QTest::newRow("single value") << "2" << true << 1 << "2" << PageRangeValidator::NoError; |
875 | + QTest::newRow("multiple values") << "2,3,4" << true << 3 << "2,3,4" << PageRangeValidator::NoError; |
876 | + QTest::newRow("multiple values sort") << "4,3,2" << true << 3 << "2,3,4" << PageRangeValidator::NoError; |
877 | + QTest::newRow("no lower") << "-5" << true << 5 << "1,2,3,4,5" << PageRangeValidator::NoError; |
878 | + QTest::newRow("no upper") << "5-" << true << 6 << "5,6,7,8,9,10" << PageRangeValidator::NoError; |
879 | + QTest::newRow("single range") << "2-3" << true << 2 << "2,3" << PageRangeValidator::NoError; |
880 | + QTest::newRow("range and value") << "2-3,6" << true << 3 << "2,3,6" << PageRangeValidator::NoError; |
881 | + QTest::newRow("multiple ranges") << "2-3,6-7" << true << 4 << "2,3,6,7" << PageRangeValidator::NoError; |
882 | + QTest::newRow("overlapping ranges") << "2-5,3-6" << true << 5 << "2,3,4,5,6" << PageRangeValidator::NoError; |
883 | + |
884 | + // Values that pass validate() but fail range checks |
885 | + QTest::newRow("dash") << "-" << false << 0 << "" << PageRangeValidator::NoPageInRange; |
886 | + QTest::newRow("comma") << "," << false << 0 << "" << PageRangeValidator::NoPageInRange; |
887 | + QTest::newRow("double comma") << ",," << false << 0 << "" << PageRangeValidator::NoPageInRange; |
888 | + |
889 | + // Values that fail range checks |
890 | + QTest::newRow("low less than 1") << "0-" << false << 0 << "" << PageRangeValidator::PageLessThanOne; |
891 | + QTest::newRow("high less than 1") << "-0" << false << 0 << "" << PageRangeValidator::PageLessThanOne; |
892 | + QTest::newRow("low higher than doc count") << "50-" << false << 0 << "" << PageRangeValidator::PageHigherThanDocument; |
893 | + QTest::newRow("high higher than doc count") << "-50" << false << 0 << "" << PageRangeValidator::PageHigherThanDocument; |
894 | + QTest::newRow("low higher than high") << "5-3" << false << 0 << "" << PageRangeValidator::PageLowGreaterThanHigh; |
895 | + } |
896 | + |
897 | + void testValidateValues() |
898 | + { |
899 | + QFETCH(QString, input); |
900 | + QFETCH(bool, result); |
901 | + QFETCH(int, count); |
902 | + QFETCH(QString, pages); |
903 | + QFETCH(PageRangeValidator::PageRangeError, error); |
904 | + |
905 | + m_validator->setDocumentCount(10); |
906 | + |
907 | + QCOMPARE(m_validator->validateValues(input), result); |
908 | + QCOMPARE(m_validator->count(), count); |
909 | + QCOMPARE(m_validator->pages(), pages); |
910 | + QCOMPARE(m_validator->error(), error); |
911 | + } |
912 | + |
913 | +private: |
914 | + PageRangeValidator *m_validator; |
915 | +}; |
916 | + |
917 | +QTEST_GUILESS_MAIN(TestPageRangeValidator) |
918 | +#include "tst_pagerangevalidator.moc" |
Page ranges shouldn't be sorted, as the user may have a valid reason for wanting to specify their pages in a different order (e.g. "1, 5, 3" should print in that order), this is the current behaviour for existing print dialogs.
Also it'd be nice to support reversed ranges, so "5-1" would print 5, 4, 3, 2, 1 in that order. Or a more complicated example could be "3-1, 5, 4" should print 3, 2, 1, 5, 4.