Merge lp:~ahayzen/ubuntu-printing-app/page-range-validate-and-count into lp:ubuntu-printing-app

[Andrew Hayzen]

* Add validation to copies and page range fields with tests

* Add validation to copies and page range fields with tests

[Michael Sheldon (michael-sheldon)]

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.

review: Needs Fixing

[Andrew Hayzen (ahayzen)]

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.

0 - https://www.cups.org/doc/options.html#PAGERANGES

[Andrew Hayzen (ahayzen)]

Reported bug 1680048 for advanced page ranges. And now setting Qt.ImhPreferNumbers for the page range field.

[Michael Sheldon (michael-sheldon)]

Looks good :)

review: Approve

Unmerged revisions

73. By Andrew Hayzen on 2017-04-05

* Set Qt.ImhPreferNumbers for page-ranges text field

72. By Andrew Hayzen on 2017-04-05

* Remove unneeded TODO

71. By Andrew Hayzen on 2017-04-05

* Ensure NoError state is always set
* Update naming of error for 5-3

70. By Andrew Hayzen on 2017-03-31

* Rebase onto lp:ubuntu-printing-app

69. By Andrew Hayzen on 2017-03-31

* Add validation to copies field with tests

68. By Andrew Hayzen on 2017-03-30

* Add page range validation and counting with tests

=== modified file 'po/ubuntu-printing-app.pot'
--- po/ubuntu-printing-app.pot	2017-03-21 17:57:19 +0000
+++ po/ubuntu-printing-app.pot	2017-04-05 12:09:01 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-03-21 17:56+0000\n"
+"POT-Creation-Date: 2017-04-05 12:51+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -185,39 +185,55 @@
 msgid "Copies"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:131
+#: ../ubuntu-printing-app/pages/PrintPage.qml:130
 msgid "All"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:131
+#: ../ubuntu-printing-app/pages/PrintPage.qml:130
 msgid "Range"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:135
+#: ../ubuntu-printing-app/pages/PrintPage.qml:134
 msgid "Pages"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:177
+#: ../ubuntu-printing-app/pages/PrintPage.qml:175
 msgid "eg 1-3,8"
 msgstr ""
 
 #: ../ubuntu-printing-app/pages/PrintPage.qml:186
+msgid "No number given in range"
+msgstr ""
+
+#: ../ubuntu-printing-app/pages/PrintPage.qml:189
+msgid "Page is less than one"
+msgstr ""
+
+#: ../ubuntu-printing-app/pages/PrintPage.qml:192
+msgid "Page is higher than document page count"
+msgstr ""
+
+#: ../ubuntu-printing-app/pages/PrintPage.qml:195
+msgid "Second page cannot be lower than first"
+msgstr ""
+
+#: ../ubuntu-printing-app/pages/PrintPage.qml:211
 msgid "Two-sided"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:207
+#: ../ubuntu-printing-app/pages/PrintPage.qml:232
 msgid "Color"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:228
+#: ../ubuntu-printing-app/pages/PrintPage.qml:253
 msgid "Quality"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:246
+#: ../ubuntu-printing-app/pages/PrintPage.qml:271
 msgid "Collate"
 msgstr ""
 
-#: ../ubuntu-printing-app/pages/PrintPage.qml:266
+#: ../ubuntu-printing-app/pages/PrintPage.qml:291
 msgid "Reverse"
 msgstr ""
 
 
=== modified file 'ubuntu-printing-app/backend/CMakeLists.txt'
--- ubuntu-printing-app/backend/CMakeLists.txt	2017-03-07 15:01:56 +0000
+++ ubuntu-printing-app/backend/CMakeLists.txt	2017-04-05 12:09:01 +0000
@@ -9,8 +9,9 @@
   UbuntuPrintingAppSRC
   UbuntuPrintingApp/backend.cpp
   UbuntuPrintingApp/document.cpp
+  UbuntuPrintingApp/pagehelper.cpp
+  UbuntuPrintingApp/pagerangevalidator.cpp
   UbuntuPrintingApp/popplerimageprovider.cpp
-  UbuntuPrintingApp/pagehelper.cpp
 )
 
 add_library(${LIBNAME} SHARED ${UbuntuPrintingAppSRC})
 
=== modified file 'ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp'
--- ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp	2017-01-30 15:19:57 +0000
+++ ubuntu-printing-app/backend/UbuntuPrintingApp/backend.cpp	2017-04-05 12:09:01 +0000
@@ -23,6 +23,7 @@
 
 #include "document.h"
 #include "pagehelper.h"
+#include "pagerangevalidator.h"
 #include "popplerimageprovider.h"
 
 void BackendPlugin::registerTypes(const char *uri)
@@ -31,6 +32,7 @@
 
     qmlRegisterType<Document>(uri, 1, 0, "Document");
     qmlRegisterType<PageHelper>(uri, 1, 0, "PageHelper");
+    qmlRegisterType<PageRangeValidator>(uri, 1, 0, "PageRangeValidator");
 }
 
 void BackendPlugin::initializeEngine(QQmlEngine *engine, const char *uri)
 
=== added file 'ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp'
--- ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp	1970-01-01 00:00:00 +0000
+++ ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.cpp	2017-04-05 12:09:01 +0000
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2017 Canonical Ltd.
+ *
+ * This file is part of ubuntu-printing-app.
+ *
+ * ubuntu-printing-app is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * ubuntu-printing-app is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored-by: Andrew Hayzen <[e-mail]>
+ */
+
+#include "pagerangevalidator.h"
+
+#include <QtCore/QRegularExpressionMatch>
+#include <QtCore/QList>
+
+PageRangeValidator::PageRangeValidator(QObject *parent)
+    : QValidator(parent)
+    , m_count(0)
+    , m_document_count(0)
+    , m_error(PageRangeError::NoError)
+    , m_pages({})
+    , m_validate_re(QStringLiteral("^(\\d+)?[-]?(\\d+)?([,](\\d+)?[-]?(\\d+)?)*$"))
+    , m_validate_values_re(QStringLiteral("^(\\d*)?[-]?(\\d*)?$"))
+{
+
+}
+
+int PageRangeValidator::count() const
+{
+    return m_count;
+}
+
+int PageRangeValidator::documentCount() const
+{
+    return m_document_count;
+}
+
+PageRangeValidator::PageRangeError PageRangeValidator::error() const
+{
+    return m_error;
+}
+
+QString PageRangeValidator::pages() const
+{
+    // Convert the pages to a list of comma's for QML and cups
+    QStringList list;
+
+    // Get a list and sort while still int so that 10 > 2
+    QList<int> pages = m_pages.toList();
+    qSort(pages);
+
+    Q_FOREACH(int page, pages) {
+        list << QString::number(page);
+    }
+
+    return list.join(",");
+}
+
+void PageRangeValidator::setCount(const int count)
+{
+    if (m_count != count) {
+        m_count = count;
+
+        Q_EMIT countChanged(m_count);
+    }
+}
+
+void PageRangeValidator::setDocumentCount(const int documentCount)
+{
+    if (m_document_count != documentCount) {
+        m_document_count = documentCount;
+
+        Q_EMIT documentCountChanged(m_document_count);
+    }
+}
+
+void PageRangeValidator::setError(const PageRangeValidator::PageRangeError error)
+{
+    if (m_error != error) {
+        m_error = error;
+
+        Q_EMIT errorChanged(m_error);
+    }
+}
+
+void PageRangeValidator::setPages(const QSet<int> pages)
+{
+    if (m_pages != pages) {
+        m_pages = pages;
+
+        setCount(m_pages.count());
+
+        Q_EMIT pagesChanged(this->pages());
+    }
+}
+
+QValidator::State PageRangeValidator::validate(QString &input, int &pos) const
+{
+    Q_UNUSED(pos);
+
+    // TODO: maybe use partial match for 1-3-5?
+    // QRegularExpression::PartialPreferCompleteMatch
+    // QValidator::State::Intermediate
+    // however then it allows 1,------
+    QRegularExpressionMatch match = m_validate_re.match(input, 0);
+
+    if (match.hasMatch()) {
+        return QValidator::State::Acceptable;
+    } else {
+        return QValidator::State::Invalid;
+    }
+}
+
+bool PageRangeValidator::validateValues(QString input)
+{
+    bool valid = true;
+
+    // If the given value is empty then set the whole range
+    if (input.isEmpty()) {
+        setPages({});
+        setCount(documentCount());
+
+        return valid;
+    }
+
+    QSet<int> pages;
+    setError(PageRangeError::NoError);
+
+    Q_FOREACH(QString part, input.split(",")) {
+        QRegularExpressionMatch matches = m_validate_values_re.match(part);
+
+        QString low = matches.captured(1);
+        QString high = matches.captured(2);
+
+        if (low.isEmpty() && high.isEmpty()) {
+            setError(PageRangeError::NoPageInRange);
+            valid = false;
+            break;
+        } else if (high.isEmpty()) {
+            if (part.contains("-")) {
+                // 1-  add the range from the lower until the document count
+                high = QString::number(documentCount());
+            } else {
+                // 1  Add the given page
+                high = low;
+            }
+        } else if (low.isEmpty()) {
+            // -3 add the range up to the higher number
+            low = QString::number(1);
+        } else {
+            // high and low set, we don't need todo anything
+        }
+
+        // Convert to ints
+        int lower = low.toInt();
+        int higher = high.toInt();
+
+        // Validate the numbers
+        if (lower < 1 || higher < 1) {
+            setError(PageRangeError::PageLessThanOne);
+            valid = false;
+            break;
+        } else if (lower > documentCount() || higher > documentCount()) {
+            setError(PageRangeError::PageHigherThanDocument);
+            valid = false;
+            break;
+        } else if (lower > higher) {
+            setError(PageRangeError::PageLowGreaterThanHigh);
+            valid = false;
+            break;
+        } else {
+            // Valid so lets push given pages numbers
+            for (int i=lower; i <= higher; i++) {
+                pages.insert(i);
+            }
+        }
+    }
+
+    setPages(pages);
+
+    return valid;
+}
 
=== added file 'ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h'
--- ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h	1970-01-01 00:00:00 +0000
+++ ubuntu-printing-app/backend/UbuntuPrintingApp/pagerangevalidator.h	2017-04-05 12:09:01 +0000
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2017 Canonical Ltd.
+ *
+ * This file is part of ubuntu-printing-app.
+ *
+ * ubuntu-printing-app is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * ubuntu-printing-app is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored-by: Andrew Hayzen <[e-mail]>
+ */
+#ifndef PAGERANGEVALIDATOR_H
+#define PAGERANGEVALIDATOR_H
+
+#include <QtCore/QRegularExpression>
+#include <QtCore/QSet>
+#include <QtGui/QValidator>
+
+class PageRangeValidator : public QValidator
+{
+    Q_OBJECT
+
+    Q_PROPERTY(int count READ count NOTIFY countChanged)
+    Q_PROPERTY(int documentCount READ documentCount WRITE setDocumentCount NOTIFY documentCountChanged)
+    Q_PROPERTY(PageRangeError error READ error NOTIFY errorChanged)
+    Q_PROPERTY(QString pages READ pages NOTIFY pagesChanged)
+public:
+    PageRangeValidator(QObject *parent=Q_NULLPTR);
+
+    enum PageRangeError {
+        NoError,
+        NoPageInRange,
+        PageLessThanOne,
+        PageHigherThanDocument,
+        PageLowGreaterThanHigh,
+    };
+    Q_ENUM(PageRangeError)
+
+    int count() const;
+    int documentCount() const;
+    PageRangeError error() const;
+    QString pages() const;
+    virtual QValidator::State validate(QString &input, int &pos) const override;
+
+    Q_INVOKABLE bool validateValues(QString input);
+Q_SIGNALS:
+    void countChanged(int count);
+    void documentCountChanged(int documentCount);
+    void errorChanged(PageRangeError error);
+    void pagesChanged(QString pages);
+public Q_SLOTS:
+    void setDocumentCount(const int documentCount);
+private Q_SLOTS:
+    void setCount(const int count);
+    void setPages(const QSet<int> pages);
+    void setError(const PageRangeError error);
+private:
+    int m_count;
+    int m_document_count;
+    PageRangeError m_error;
+    QSet<int> m_pages;
+    QRegularExpression m_validate_re;
+    QRegularExpression m_validate_values_re;
+};
+
+
+#endif // PAGERANGEVALIDATOR_H
 
=== modified file 'ubuntu-printing-app/components/LabelRow.qml'
--- ubuntu-printing-app/components/LabelRow.qml	2017-03-07 13:46:02 +0000
+++ ubuntu-printing-app/components/LabelRow.qml	2017-04-05 12:09:01 +0000
@@ -26,6 +26,7 @@
     property alias enabled: secondaryLabel.enabled
     property alias primaryText: primaryLabel.text
     property alias secondaryText: secondaryLabel.text
+    property alias secondaryColor: secondaryLabel.color
 
     Label {
         id: primaryLabel
@@ -40,5 +41,6 @@
         Layout.preferredWidth: units.gu(10)
         objectName: "secondary"
         verticalAlignment: Text.AlignVCenter
+        wrapMode: Text.WrapAtWordBoundaryOrAnywhere
     }
 }
 
=== modified file 'ubuntu-printing-app/pages/PrintPage.qml'
--- ubuntu-printing-app/pages/PrintPage.qml	2017-03-21 18:00:19 +0000
+++ ubuntu-printing-app/pages/PrintPage.qml	2017-04-05 12:09:01 +0000
@@ -105,9 +105,8 @@
                 inputMethodHints: Qt.ImhDigitsOnly
                 objectName: "copiesTextField"
                 text: i18n.tr("Copies")
-                validator: IntValidator {
-                    bottom: 1
-                    top: 999
+                validator: RegExpValidator {
+                    regExp: /^[1-9](\d)*$/
                 }
                 value: printing.printerJob.copies
 
@@ -151,23 +150,23 @@
             TextFieldRow {
                 id: pageRangeTextField
                 enabled: printing.isLoaded && !printing.pdfMode
+                inputMethodHints: Qt.ImhPreferNumbers
                 objectName: "pageRangeTextField"
-                validator: RegExpValidator {
-//                        regExp: ""  // TODO: validate to only 0-9||9-0||0 ,
+                validator: PageRangeValidator {
+                    documentCount: document.count
+
+                    // validate() is const, so bind a secondary validateValues
+                    // which checks the values in the range are valid
+                    // and sets the count/pages
+                    readonly property bool validValues: validateValues(pageRangeTextField.value)
                 }
                 visible: pageRangeSelector.selectedValue === PrinterEnum.PageRange
 
-                onValueChanged: {
-                    if (printing.printerJob.printRange !== value) {
-                        printing.printerJob.printRange = value
-                    }
-                }
-
                 Binding {
-                    target: pageRangeTextField
-                    property: "value"
-                    when: printing.printerJob && pageRangeTextField.enabled
-                    value: printing.printerJob.printRange
+                    target: printing.printerJob
+                    property: "printRange"
+                    when: printing.printerJob && pageRangeTextField.enabled && pageRangeTextField.validator.validValues
+                    value: pageRangeTextField.validator.pages
                 }
             }
 
@@ -178,6 +177,33 @@
                 visible: pageRangeSelector.selectedValue === PrinterEnum.PageRange
             }
 
+            LabelRow {
+                enabled: printing.isLoaded && !printing.pdfMode
+                objectName: "pageRangeErrorStrLabel"
+                secondaryColor: theme.palette.normal.negative
+                secondaryText: {
+                    switch (pageRangeTextField.validator.error) {
+                    case PageRangeValidator.NoPageInRange:
+                        i18n.tr("No number given in range")
+                        break;
+                    case PageRangeValidator.PageLessThanOne:
+                        i18n.tr("Page is less than one")
+                        break;
+                    case PageRangeValidator.PageHigherThanDocument:
+                        i18n.tr("Page is higher than document page count")
+                        break;
+                    case PageRangeValidator.PageLowGreaterThanHigh:
+                        i18n.tr("Second page cannot be lower than first")
+                        break;
+                    default:
+                    case PageRangeValidator.NoError:
+                        ""
+                        break;
+                    }
+                }
+                visible: pageRangeSelector.selectedValue === PrinterEnum.PageRange && !pageRangeTextField.validator.validValues
+            }
+
             SelectorRow {
                 id: duplexSelector
                 enabled: printing.isEditable && currentDocument.count > 1 && printing.printer.supportedDuplexModes.length > 1
@@ -297,16 +323,22 @@
             right: parent.right
             rightMargin: units.gu(1)
         }
-        canPrint: printing.isLoaded
+        canPrint: {
+            if (printing.pdfMode) {
+                printing.isLoaded
+            } else if (printing.printerJob.printRangeMode === PrinterEnum.PageRange) {
+                // If the page range is set, then check it is valid
+                printing.isLoaded && copiesSelector.acceptableInput && pageRangeTextField.validator.validValues
+            } else {
+                printing.isLoaded && copiesSelector.acceptableInput
+            }
+        }
         objectName: "printRow"
         pdfMode: printing.pdfMode
-        // TODO: This should count the range not all pages
-        // roundUp((pageCount * copies) / duplex)
         sheets: {
-            // If range is selected and a value exists, then set the sheets to zero
-            // for now. Which results in not showing the number sheets
+            // If the page range is set then use it
             if (printing.printerJob.printRangeMode === PrinterEnum.PageRange && printing.printerJob.printRange.length > 0) {
-                0
+                Math.ceil((pageRangeTextField.validator.count * printing.printerJob.copies) / (printing.printerJob.isTwoSided ? 2 : 1))
             } else {
                 Math.ceil((document.count * printing.printerJob.copies) / (printing.printerJob.isTwoSided ? 2 : 1))
             }
 
=== modified file 'ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml'
--- ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml	2017-03-17 12:34:49 +0000
+++ ubuntu-printing-app/tests/qmltests/tst_LabelRow.qml	2017-04-05 12:09:01 +0000
@@ -36,11 +36,13 @@
 
         readonly property bool dataEnabled: true
         readonly property string dataPrimaryText: "Primary"
+        readonly property string dataSecondaryColor: "#ff0000"
         readonly property string dataSecondaryText: "Secondary"
 
         function init() {
             labelRow.enabled = dataEnabled;
             labelRow.primaryText = dataPrimaryText;
+            labelRow.secondaryColor = dataSecondaryColor;
             labelRow.secondaryText = dataSecondaryText;
 
             waitForRendering(labelRow);
@@ -69,5 +71,11 @@
             var secondary = findChild(labelRow, "secondary");
             compare(secondary.text, dataSecondaryText);
         }
+
+        function test_secondaryColor() {
+            // Check secondary color is correct
+            var secondary = findChild(labelRow, "secondary");
+            compare(secondary.color, dataSecondaryColor);
+        }
     }
 }
 
=== modified file 'ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml'
--- ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml	2017-03-21 17:57:19 +0000
+++ ubuntu-printing-app/tests/qmltests/tst_PrintPage.qml	2017-04-05 12:09:01 +0000
@@ -119,11 +119,15 @@
 
             mockPrinting.printerJob.collate = true;
             mockPrinting.printerJob.copies = 1;
+            mockPrinting.printerJob.isTwoSided = false;
             mockPrinting.printerJob.printRangeMode = PrinterEnum.AllPages;
             mockPrinting.printerJob.reverse = false;
 
             mockPrinting.printerSelectedIndex = 0;
 
+            var printRange = findChild(printPage, "pageRangeTextField");
+            printRange.value = "";
+
             cancelSpy.clear();
             confirmSpy.clear();
 
@@ -261,6 +265,24 @@
             compare(mockPrinting.printerJob.copies, 2);
         }
 
+        function test_copiesKeyClickZero() {
+            // Check copies starting value is correct
+            var copies = findChild(printPage, "copiesTextField");
+            compare(copies.value, "1");
+
+            // Click on the textField
+            mouseClick(copies);
+
+            // Clear the current text and try to enter "01"
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_0);
+            keyClick(Qt.Key_1);
+
+            // Check that "1" is set to the backend
+            tryCompare(copies, "value", "1", timeout, "Copies value did not change");
+            compare(mockPrinting.printerJob.copies, 1);
+        }
+
         function test_duplex() {
             var duplex = findChild(printPage, "duplexSelector");
 
@@ -406,6 +428,73 @@
             }
         }
 
+        function test_printRange() {
+            document.url = Qt.resolvedUrl("../resources/pdf/mixed_portrait.pdf");
+
+            // Change to PageRange mode
+            mockPrinting.printerJob.printRangeMode = PrinterEnum.PageRange;
+
+            var printRange = findChild(printPage, "pageRangeTextField");
+            var printRangeError = findChild(printPage, "pageRangeErrorStrLabel");
+            var printRow = findChild(printPage, "printRow");
+
+            // Test that a range sets printRange in printerJob
+
+            // Click on the textField
+            mouseClick(printRange);
+
+            // Clear the current text and enter "2"
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_2);
+
+            compare(mockPrinting.printerJob.printRange, "2");
+            compare(printRangeError.visible, false);
+            compare(printRangeError.secondaryText, "");
+            compare(printRow.canPrint, true);
+
+
+            // Test that a invalid range shows the error and disables button
+
+            // Click on the textField
+            mouseClick(printRange);
+
+            // Clear the current text and enter ","
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_Comma);
+
+            compare(printRangeError.visible, true);
+            compare(printRangeError.secondaryText, i18n.tr("No number given in range"));
+            compare(printRow.canPrint, false);
+
+            // Clear the current text and enter "0"
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_0);
+
+            compare(printRangeError.visible, true);
+            compare(printRangeError.secondaryText, i18n.tr("Page is less than one"));
+            compare(printRow.canPrint, false);
+
+            // Clear the current text and enter "10"
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_1);
+            keyClick(Qt.Key_0);
+
+            compare(printRangeError.visible, true);
+            compare(printRangeError.secondaryText, i18n.tr("Page is higher than document page count"));
+            compare(printRow.canPrint, false);
+
+            // Clear the current text and enter "2-1"
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_Backspace);
+            keyClick(Qt.Key_2);
+            keyClick(Qt.Key_Minus);
+            keyClick(Qt.Key_1);
+
+            compare(printRangeError.visible, true);
+            compare(printRangeError.secondaryText, i18n.tr("Page low is greater than high"));
+            compare(printRow.canPrint, false);
+        }
+
         function test_printRangeMode() {
             var printRangeMode = findChild(printPage, "pageRangeSelector");
             var printRange = findChild(printPage, "pageRangeTextField");
@@ -503,12 +592,70 @@
             mockPrinting.printerJob.isTwoSided = true;
 
             compare(printRow.sheets, 2);
-
-            // Enable page ranges, this results in zero sheets
-            mockPrinting.printerJob.printRange = "-3,5-7,9-";
+        }
+
+        function test_sheets_range() {
+            var printRow = findChild(printPage, "printRow");
+            var printRange = findChild(printPage, "pageRangeTextField");
+
+            // has 3 pages
+            document.url = Qt.resolvedUrl("../resources/pdf/mixed_portrait.pdf");
+
+            compare(document.count, 3);
+            compare(mockPrinting.printerJob.copies, 1);
+            compare(mockPrinting.printerJob.isTwoSided, false);
+            compare(printRow.sheets, 3);
+
+            // Set range to 1 check than sheets is correct
             mockPrinting.printerJob.printRangeMode = PrinterEnum.PageRange;
-
-            compare(printRow.sheets, 0);
+            printRange.value = "1";
+
+            compare(printRow.sheets, 1);
+
+            // Enable twoSided, we should get 0.5 rounded to 1 sheet
+            mockPrinting.printerJob.isTwoSided = true;
+
+            compare(mockPrinting.printerJob.isTwoSided, true);
+            compare(printRow.sheets, 1);
+
+            // Enable two copies, we should get 2 copies with duplex, so 1 sheet
+            mockPrinting.printerJob.copies = 2;
+
+            compare(mockPrinting.printerJob.copies, 2);
+            compare(printRow.sheets, 1);
+
+            // Disable two sided, so 2 copies and 2 sheets
+            mockPrinting.printerJob.isTwoSided = false;
+
+            compare(mockPrinting.printerJob.isTwoSided, false);
+            compare(printRow.sheets, 2);
+
+
+            // Set the range to 2 sheets
+            printRange.value = "1,2";
+
+            mockPrinting.printerJob.copies = 1;
+            mockPrinting.printerJob.isTwoSided = false;
+
+            compare(printRow.sheets, 2);
+
+            // Enable twoSided, we should get 2 pages on 1 sheet
+            mockPrinting.printerJob.isTwoSided = true;
+
+            compare(mockPrinting.printerJob.isTwoSided, true);
+            compare(printRow.sheets, 1);
+
+            // Enable two copies, we should get 2 copies with duplex, so 2 sheet
+            mockPrinting.printerJob.copies = 2;
+
+            compare(mockPrinting.printerJob.copies, 2);
+            compare(printRow.sheets, 2);
+
+            // Disable two sided, so 2 copies and 4 sheets
+            mockPrinting.printerJob.isTwoSided = false;
+
+            compare(mockPrinting.printerJob.isTwoSided, false);
+            compare(printRow.sheets, 4);
         }
     }
 }
 
=== modified file 'ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt'
--- ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt	2017-03-15 11:57:40 +0000
+++ ubuntu-printing-app/tests/unittests/backend/CMakeLists.txt	2017-04-05 12:09:01 +0000
@@ -14,6 +14,10 @@
 target_link_libraries(testPageHelper UbuntuPrintingAppbackend Qt5::Test Qt5::Gui)
 add_test(tst_pagehelper testPageHelper)
 
+add_executable(testPageRangeValidator tst_pagerangevalidator.cpp ${PDF_TEST_FILES})
+target_link_libraries(testPageRangeValidator UbuntuPrintingAppbackend Qt5::Test Qt5::Gui)
+add_test(tst_pagerangevalidator testPageRangeValidator)
+
 add_executable(testPopplerImageProvider tst_popplerimageprovider.cpp ${PDF_TEST_FILES})
 target_link_libraries(testPopplerImageProvider UbuntuPrintingAppbackend Qt5::Test Qt5::Gui)
 add_test(tst_popplerimageprovider testPopplerImageProvider)
 
=== added file 'ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp'
--- ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp	1970-01-01 00:00:00 +0000
+++ ubuntu-printing-app/tests/unittests/backend/tst_pagerangevalidator.cpp	2017-04-05 12:09:01 +0000
@@ -0,0 +1,151 @@
+/*
+ * Copyright 2017 Canonical Ltd.
+ *
+ * This file is part of ubuntu-printing-app.
+ *
+ * ubuntu-printing-app is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * ubuntu-printing-app is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored-by: Andrew Hayzen <[e-mail]>
+ */
+
+#include <QObject>
+#include <QSignalSpy>
+#include <QTest>
+
+#include "UbuntuPrintingApp/pagerangevalidator.h"
+
+class TestPageRangeValidator : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void init()
+    {
+        m_validator = new PageRangeValidator();
+    }
+    void cleanup()
+    {
+        QSignalSpy destroyedSpy(m_validator, SIGNAL(destroyed(QObject*)));
+        m_validator->deleteLater();
+        QTRY_COMPARE(destroyedSpy.count(), 1);
+    }
+
+    // Check that the initial values are correct
+    void testInit()
+    {
+        QCOMPARE(m_validator->count(), 0);
+        QCOMPARE(m_validator->documentCount(), 0);
+        QCOMPARE(m_validator->error(), PageRangeValidator::PageRangeError::NoError);
+        QCOMPARE(m_validator->pages(), QStringLiteral(""));
+    }
+
+    // Check that the document count can be set
+    void testDocumentCount()
+    {
+        QSignalSpy documentCountSpy(m_validator, SIGNAL(documentCountChanged(int)));
+        m_validator->setDocumentCount(10);
+
+        QTRY_COMPARE(documentCountSpy.count(), 1);
+        QCOMPARE(m_validator->documentCount(), 10);
+    }
+
+    void testValidate_data()
+    {
+        QTest::addColumn<QString>("input");
+        QTest::addColumn<bool>("result");
+
+        // Good values
+        QTest::newRow("empty") << "" << true;
+        QTest::newRow("single value") << "2" << true;
+        QTest::newRow("multiple values") << "2,3,4" << true;
+        QTest::newRow("no lower") << "-2" << true;
+        QTest::newRow("no upper") << "2-" << true;
+        QTest::newRow("single range") << "2-3" << true;
+        QTest::newRow("range and value") << "2-3,6" << true;
+        QTest::newRow("multiple ranges") << "2-3,6-7" << true;
+        QTest::newRow("overlapping ranges") << "2-8,6-10" << true;
+        QTest::newRow("dash") << "-" << true;
+        QTest::newRow("comma") << "," << true;
+        QTest::newRow("double comma") << ",," << true;
+
+        // Bad values
+        QTest::newRow("no comma multi range") << "1-2-3" << false;
+        QTest::newRow("double dash") << "--" << false;
+        QTest::newRow("double dash range") << "2--3" << false;
+        QTest::newRow("letter") << "a" << false;
+    }
+
+    void testValidate()
+    {
+        QFETCH(QString, input);
+        QFETCH(bool, result);
+
+        int pos = 0;
+
+        QCOMPARE(m_validator->validate(input, pos), result ? QValidator::State::Acceptable : QValidator::State::Invalid);
+    }
+
+    void testValidateValues_data()
+    {
+        QTest::addColumn<QString>("input");
+        QTest::addColumn<bool>("result");
+        QTest::addColumn<int>("count");
+        QTest::addColumn<QString>("pages");
+        QTest::addColumn<PageRangeValidator::PageRangeError>("error");
+
+        // Good values
+        QTest::newRow("empty") << "" << true << 10 << "" << PageRangeValidator::NoError;
+        QTest::newRow("single value") << "2" << true << 1 << "2" << PageRangeValidator::NoError;
+        QTest::newRow("multiple values") << "2,3,4" << true << 3 << "2,3,4" << PageRangeValidator::NoError;
+        QTest::newRow("multiple values sort") << "4,3,2" << true << 3 << "2,3,4" << PageRangeValidator::NoError;
+        QTest::newRow("no lower") << "-5" << true << 5 << "1,2,3,4,5" << PageRangeValidator::NoError;
+        QTest::newRow("no upper") << "5-" << true << 6 << "5,6,7,8,9,10" << PageRangeValidator::NoError;
+        QTest::newRow("single range") << "2-3" << true << 2 << "2,3" << PageRangeValidator::NoError;
+        QTest::newRow("range and value") << "2-3,6" << true << 3 << "2,3,6" << PageRangeValidator::NoError;
+        QTest::newRow("multiple ranges") << "2-3,6-7" << true << 4 << "2,3,6,7" << PageRangeValidator::NoError;
+        QTest::newRow("overlapping ranges") << "2-5,3-6" << true << 5 << "2,3,4,5,6" << PageRangeValidator::NoError;
+
+        // Values that pass validate() but fail range checks
+        QTest::newRow("dash") << "-" << false << 0 << "" << PageRangeValidator::NoPageInRange;
+        QTest::newRow("comma") << "," << false << 0 << "" << PageRangeValidator::NoPageInRange;
+        QTest::newRow("double comma") << ",," << false << 0 << "" << PageRangeValidator::NoPageInRange;
+
+        // Values that fail range checks
+        QTest::newRow("low less than 1") << "0-" << false << 0 << "" << PageRangeValidator::PageLessThanOne;
+        QTest::newRow("high less than 1") << "-0" << false << 0 << "" << PageRangeValidator::PageLessThanOne;
+        QTest::newRow("low higher than doc count") << "50-" << false << 0 << "" << PageRangeValidator::PageHigherThanDocument;
+        QTest::newRow("high higher than doc count") << "-50" << false << 0 << "" << PageRangeValidator::PageHigherThanDocument;
+        QTest::newRow("low higher than high") << "5-3" << false << 0 << "" << PageRangeValidator::PageLowGreaterThanHigh;
+    }
+
+    void testValidateValues()
+    {
+        QFETCH(QString, input);
+        QFETCH(bool, result);
+        QFETCH(int, count);
+        QFETCH(QString, pages);
+        QFETCH(PageRangeValidator::PageRangeError, error);
+
+        m_validator->setDocumentCount(10);
+
+        QCOMPARE(m_validator->validateValues(input), result);
+        QCOMPARE(m_validator->count(), count);
+        QCOMPARE(m_validator->pages(), pages);
+        QCOMPARE(m_validator->error(), error);
+    }
+
+private:
+    PageRangeValidator *m_validator;
+};
+
+QTEST_GUILESS_MAIN(TestPageRangeValidator)
+#include "tst_pagerangevalidator.moc"