PdfMultiPageView: use TableView; horz. scroll; control page position

TableView is missing some features compared to ListView; so finding out
where we currently are (which row) and programmatic positioning on a
specific y coordinate of a specific row require some workarounds for now,
including helpers in PdfDocument.

TableView also assumes (and sporadically enforces) that all cells in a
column have the same width.  So we need a placeholder Item for each page.
This also helps with rotation: the placeholder is now as wide as the
window or the image, whichever is wider, and the "paper" is centered
within; thus there's always room to rotate it.

There's still some problem with setting contentY in goToPage() after
the page has been zoomed to a size larger than the window: the values
look correct, but it scrolls too far.

But on the plus side, horizontal scrolling works.  So now we attempt to
control the horizontal position too: NavigationStack tracks it, and can
go back to a previous position; and links can in theory jump to specific
positions and zoom levels, scrolling horizontally such that a specific x
coordinate is visible.

Includes minor UI tweaks to make it look better on iOS.

Change-Id: I643d8ef48ef815aeb49cae77dcb84c3682563d56
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

5 files changed, 283 insertions, 168 deletions

diff --git a/examples/pdf/multipage/viewer.qml b/examples/pdf/multipage/viewer.qml
index ac6d2cd9a..9e5f92407 100644
--- a/examples/pdf/multipage/viewer.qml
+++ b/examples/pdf/multipage/viewer.qml
@@ -249,6 +249,7 @@ ApplicationWindow {
         y: root.header.height
         height: view.height
         dim: false
+        clip: true
         ListView {
             id: searchResultsList
             anchors.fill: parent
@@ -286,7 +287,7 @@ ApplicationWindow {
             TextField {
                 id: searchField
                 placeholderText: "search"
-                Layout.minimumWidth: 200
+                Layout.minimumWidth: 150
                 Layout.fillWidth: true
                 onAccepted: searchDrawer.open()
                 Image {
@@ -316,11 +317,10 @@ ApplicationWindow {
             }
             Label {
                 id: statusLabel
-                Layout.fillWidth: true
                 property size implicitPointSize: document.pagePointSize(view.currentPage)
                 text: "page " + (currentPageSB.value) + " of " + document.pageCount +
                       " scale " + view.renderScale.toFixed(2) +
-                      " original size " + implicitPointSize.width.toFixed(1) + "x" + implicitPointSize.height.toFixed(1) + " pt"
+                      " original " + implicitPointSize.width.toFixed(1) + "x" + implicitPointSize.height.toFixed(1) + " pt"
                 visible: document.pageCount > 0
             }
         }
diff --git a/src/pdf/quick/qml/PdfMultiPageView.qml b/src/pdf/quick/qml/PdfMultiPageView.qml
index b4bc61c64..e8eccaf3b 100644
--- a/src/pdf/quick/qml/PdfMultiPageView.qml
+++ b/src/pdf/quick/qml/PdfMultiPageView.qml
@@ -58,11 +58,15 @@ Item {
     // public API
     // TODO 5.15: required property
     property var document: undefined
+    property bool debug: false
 
     property string selectedText
     function copySelectionToClipboard() {
-        if (listView.currentItem !== null)
-            listView.currentItem.selection.copyToClipboard()
+        var currentItem = tableView.itemAtPos(0, tableView.contentY + root.height / 2)
+        if (debug)
+            console.log("currentItem", currentItem, "sel", currentItem.selection.text)
+        if (currentItem !== null)
+            currentItem.selection.copyToClipboard()
     }
 
     // page navigation
@@ -71,7 +75,11 @@ Item {
     property alias forwardEnabled: navigationStack.forwardAvailable
     function back() { navigationStack.back() }
     function forward() { navigationStack.forward() }
-    function goToPage(page) { goToLocation(page, Qt.point(0, 0), 0) }
+    function goToPage(page) {
+        if (page === navigationStack.currentPage)
+            return
+        goToLocation(page, Qt.point(0, 0), 0)
+    }
     function goToLocation(page, location, zoom) {
         if (zoom > 0)
             root.renderScale = zoom
@@ -83,22 +91,22 @@ Item {
     property real pageRotation: 0
     function resetScale() { root.renderScale = 1 }
     function scaleToWidth(width, height) {
-        root.renderScale = width / (listView.rot90 ? listView.firstPagePointSize.height : listView.firstPagePointSize.width)
+        root.renderScale = width / (tableView.rot90 ? tableView.firstPagePointSize.height : tableView.firstPagePointSize.width)
     }
     function scaleToPage(width, height) {
         var windowAspect = width / height
-        var pageAspect = listView.firstPagePointSize.width / listView.firstPagePointSize.height
-        if (listView.rot90) {
+        var pageAspect = tableView.firstPagePointSize.width / tableView.firstPagePointSize.height
+        if (tableView.rot90) {
             if (windowAspect > pageAspect) {
-                root.renderScale = height / listView.firstPagePointSize.width
+                root.renderScale = height / tableView.firstPagePointSize.width
             } else {
-                root.renderScale = width / listView.firstPagePointSize.height
+                root.renderScale = width / tableView.firstPagePointSize.height
             }
         } else {
             if (windowAspect > pageAspect) {
-                root.renderScale = height / listView.firstPagePointSize.height
+                root.renderScale = height / tableView.firstPagePointSize.height
             } else {
-                root.renderScale = width / listView.firstPagePointSize.width
+                root.renderScale = width / tableView.firstPagePointSize.width
             }
         }
     }
@@ -110,75 +118,170 @@ Item {
     function searchForward() { ++searchModel.currentResult }
 
     id: root
-    ListView {
-        id: listView
+    TableView {
+        id: tableView
         anchors.fill: parent
         model: root.document === undefined ? 0 : root.document.pageCount
-        spacing: 6
-        highlightRangeMode: ListView.ApplyRange
-        highlightMoveVelocity: 2000 // TODO increase velocity when setting currentIndex somehow, too
+        rowSpacing: 6
         property real rotationModulus: Math.abs(root.pageRotation % 180)
         property bool rot90: rotationModulus > 45 && rotationModulus < 135
+        onRot90Changed: forceLayout()
         property size firstPagePointSize: document === undefined ? Qt.size(0, 0) : document.pagePointSize(0)
+        contentWidth: document === undefined ? 0 : document.maxPageWidth * root.renderScale
+        // workaround for missing function (see https://codereview.qt-project.org/c/qt/qtdeclarative/+/248464)
+        function itemAtPos(x, y, includeSpacing) {
+            // we don't care about x (assume col 0), and assume includeSpacing is true
+            var ret = null
+            for (var i = 0; i < contentItem.children.length; ++i) {
+                var child = contentItem.children[i];
+                if (root.debug)
+                    console.log(child, "@y", child.y)
+                if (child.y < y && (!ret || child.y > ret.y))
+                    ret = child
+            }
+            if (root.debug)
+                console.log("given y", y, "found", ret, "@", ret.y)
+            return ret // the delegate with the largest y that is less than the given y
+        }
+        rowHeightProvider: function(row) { return (rot90 ? document.pagePointSize(row).width : document.pagePointSize(row).height) * root.renderScale }
         delegate: Rectangle {
-            id: paper
-            implicitWidth: image.width
-            implicitHeight: image.height
-            rotation: root.pageRotation
+            id: pageHolder
+            color: root.debug ? "beige" : "transparent"
+            Text {
+                visible: root.debug
+                anchors { right: parent.right; verticalCenter: parent.verticalCenter }
+                rotation: -90; text: pageHolder.width.toFixed(1) + "x" + pageHolder.height.toFixed(1) + "\n" +
+                                     image.width.toFixed(1) + "x" + image.height.toFixed(1)
+            }
+            implicitWidth: Math.max(root.width, (tableView.rot90 ? document.maxPageHeight : document.maxPageWidth) * root.renderScale)
+            implicitHeight: tableView.rot90 ? image.width : image.height
+            onImplicitWidthChanged: tableView.forceLayout()
+            objectName: "page " + index
+            property int delegateIndex: row // expose the context property for JS outside of the delegate
             property alias selection: selection
-            property size pagePointSize: document.pagePointSize(index)
-            property real pageScale: image.paintedWidth / pagePointSize.width
-            Image {
-                id: image
-                source: document.source
-                currentFrame: index
-                asynchronous: true
-                fillMode: Image.PreserveAspectFit
-                width: pagePointSize.width * root.renderScale
-                height: pagePointSize.height * root.renderScale
-                property real renderScale: root.renderScale
-                property real oldRenderScale: 1
-                onRenderScaleChanged: {
-                    image.sourceSize.width = pagePointSize.width * renderScale
-                    image.sourceSize.height = 0
-                    paper.scale = 1
-                    paper.x = 0
-                    paper.y = 0
+            Rectangle {
+                id: paper
+                width: image.width
+                height: image.height
+                rotation: root.pageRotation
+                anchors.centerIn: parent
+                property size pagePointSize: document.pagePointSize(index)
+                property real pageScale: image.paintedWidth / pagePointSize.width
+                Image {
+                    id: image
+                    source: document.source
+                    currentFrame: index
+                    asynchronous: true
+                    fillMode: Image.PreserveAspectFit
+                    width: paper.pagePointSize.width * root.renderScale
+                    height: paper.pagePointSize.height * root.renderScale
+                    property real renderScale: root.renderScale
+                    property real oldRenderScale: 1
+                    onRenderScaleChanged: {
+                        image.sourceSize.width = paper.pagePointSize.width * renderScale
+                        image.sourceSize.height = 0
+                        paper.scale = 1
+                    }
                 }
-            }
-            Shape {
-                anchors.fill: parent
-                opacity: 0.25
-                visible: image.status === Image.Ready
-                ShapePath {
-                    strokeWidth: 1
-                    strokeColor: "cyan"
-                    fillColor: "steelblue"
-                    scale: Qt.size(paper.pageScale, paper.pageScale)
-                    PathMultiline {
-                        paths: searchModel.boundingPolygonsOnPage(index)
+                Shape {
+                    anchors.fill: parent
+                    opacity: 0.25
+                    visible: image.status === Image.Ready
+                    ShapePath {
+                        strokeWidth: 1
+                        strokeColor: "cyan"
+                        fillColor: "steelblue"
+                        scale: Qt.size(paper.pageScale, paper.pageScale)
+                        PathMultiline {
+                            paths: searchModel.boundingPolygonsOnPage(index)
+                        }
+                    }
+                    ShapePath {
+                        fillColor: "orange"
+                        scale: Qt.size(paper.pageScale, paper.pageScale)
+                        PathMultiline {
+                            id: selectionBoundaries
+                            paths: selection.geometry
+                        }
                     }
                 }
-                ShapePath {
-                    fillColor: "orange"
-                    scale: Qt.size(paper.pageScale, paper.pageScale)
-                    PathMultiline {
-                        id: selectionBoundaries
-                        paths: selection.geometry
+                Shape {
+                    anchors.fill: parent
+                    opacity: 0.5
+                    visible: image.status === Image.Ready && searchModel.currentPage === index
+                    ShapePath {
+                        strokeWidth: 1
+                        strokeColor: "blue"
+                        fillColor: "cyan"
+                        scale: Qt.size(paper.pageScale, paper.pageScale)
+                        PathMultiline {
+                            paths: searchModel.currentResultBoundingPolygons
+                        }
                     }
                 }
-            }
-            Shape {
-                anchors.fill: parent
-                opacity: 0.5
-                visible: image.status === Image.Ready && searchModel.currentPage === index
-                ShapePath {
-                    strokeWidth: 1
-                    strokeColor: "blue"
-                    fillColor: "cyan"
-                    scale: Qt.size(paper.pageScale, paper.pageScale)
-                    PathMultiline {
-                        paths: searchModel.currentResultBoundingPolygons
+                PinchHandler {
+                    id: pinch
+                    minimumScale: 0.1
+                    maximumScale: root.renderScale < 4 ? 2 : 1
+                    minimumRotation: 0
+                    maximumRotation: 0
+                    enabled: image.sourceSize.width < 5000
+                    onActiveChanged:
+                        if (active) {
+                            paper.z = 10
+                        } else {
+                            paper.z = 0
+                            var newSourceWidth = image.sourceSize.width * paper.scale
+                            var ratio = newSourceWidth / image.sourceSize.width
+                            if (ratio > 1.1 || ratio < 0.9) {
+                                paper.scale = 1
+                                root.renderScale *= ratio
+                            }
+                        }
+                    grabPermissions: PointerHandler.CanTakeOverFromAnything
+                }
+                DragHandler {
+                    id: textSelectionDrag
+                    acceptedDevices: PointerDevice.Mouse | PointerDevice.Stylus
+                    target: null
+                }
+                TapHandler {
+                    id: tapHandler
+                    acceptedDevices: PointerDevice.Mouse | PointerDevice.Stylus
+                }
+                Repeater {
+                    model: PdfLinkModel {
+                        id: linkModel
+                        document: root.document
+                        page: image.currentFrame
+                    }
+                    delegate: Rectangle {
+                        color: "transparent"
+                        border.color: "lightgrey"
+                        x: rect.x * paper.pageScale
+                        y: rect.y * paper.pageScale
+                        width: rect.width * paper.pageScale
+                        height: rect.height * paper.pageScale
+                        MouseArea { // TODO switch to TapHandler / HoverHandler in 5.15
+                            id: linkMA
+                            anchors.fill: parent
+                            cursorShape: Qt.PointingHandCursor
+                            hoverEnabled: true
+                            onClicked: {
+                                if (page >= 0)
+                                    root.goToLocation(page, location, zoom)
+                                else
+                                    Qt.openUrlExternally(url)
+                            }
+                        }
+                        ToolTip {
+                            visible: linkMA.containsMouse
+                            delay: 1000
+                            text: page >= 0 ?
+                                      ("page " + (page + 1) +
+                                       " location " + location.x.toFixed(1) + ", " + location.y.toFixed(1) +
+                                       " zoom " + zoom) : url
+                        }
                     }
                 }
             }
@@ -186,110 +289,50 @@ Item {
                 id: selection
                 document: root.document
                 page: image.currentFrame
-                fromPoint: Qt.point(textSelectionDrag.centroid.pressPosition.x / paper.pageScale, textSelectionDrag.centroid.pressPosition.y / paper.pageScale)
-                toPoint: Qt.point(textSelectionDrag.centroid.position.x / paper.pageScale, textSelectionDrag.centroid.position.y / paper.pageScale)
+                fromPoint: Qt.point(textSelectionDrag.centroid.pressPosition.x / paper.pageScale,
+                                    textSelectionDrag.centroid.pressPosition.y / paper.pageScale)
+                toPoint: Qt.point(textSelectionDrag.centroid.position.x / paper.pageScale,
+                                  textSelectionDrag.centroid.position.y / paper.pageScale)
                 hold: !textSelectionDrag.active && !tapHandler.pressed
                 onTextChanged: root.selectedText = text
             }
-            function reRenderIfNecessary() {
-                var newSourceWidth = image.sourceSize.width * paper.scale
-                var ratio = newSourceWidth / image.sourceSize.width
-                if (ratio > 1.1 || ratio < 0.9) {
-                    image.sourceSize.height = 0
-                    image.sourceSize.width = newSourceWidth
-                    paper.scale = 1
-                }
-            }
-            PinchHandler {
-                id: pinch
-                minimumScale: 0.1
-                maximumScale: 10
-                minimumRotation: 0
-                maximumRotation: 0
-                onActiveChanged:
-                    if (active) {
-                        paper.z = 10
-                    } else {
-                        paper.x = 0
-                        paper.y = 0
-                        paper.z = 0
-                        image.width = undefined
-                        image.height = undefined
-                        paper.reRenderIfNecessary()
-                    }
-                grabPermissions: PointerHandler.CanTakeOverFromAnything
-            }
-            DragHandler {
-                id: textSelectionDrag
-                acceptedDevices: PointerDevice.Mouse | PointerDevice.Stylus
-                target: null
-            }
-            TapHandler {
-                id: tapHandler
-                acceptedDevices: PointerDevice.Mouse | PointerDevice.Stylus
-            }
-            Repeater {
-                model: PdfLinkModel {
-                    id: linkModel
-                    document: root.document
-                    page: image.currentFrame
-                }
-                delegate: Rectangle {
-                    color: "transparent"
-                    border.color: "lightgrey"
-                    x: rect.x * paper.pageScale
-                    y: rect.y * paper.pageScale
-                    width: rect.width * paper.pageScale
-                    height: rect.height * paper.pageScale
-                    MouseArea { // TODO switch to TapHandler / HoverHandler in 5.15
-                        id: linkMA
-                        anchors.fill: parent
-                        cursorShape: Qt.PointingHandCursor
-                        hoverEnabled: true
-                        onClicked: {
-                            if (page >= 0)
-                                root.goToLocation(page, location, zoom)
-                            else
-                                Qt.openUrlExternally(url)
-                        }
-                    }
-                    ToolTip {
-                        visible: linkMA.containsMouse
-                        delay: 1000
-                        text: page >= 0 ?
-                                  ("page " + (page + 1) +
-                                   " location " + location.x.toFixed(1) + ", " + location.y.toFixed(1) +
-                                   " zoom " + zoom) : url
-                    }
-                }
-            }
         }
         ScrollBar.vertical: ScrollBar {
             property bool moved: false
             onPositionChanged: moved = true
             onActiveChanged: {
-                var currentPage = listView.indexAt(0, listView.contentY)
-                var currentItem = listView.itemAtIndex(currentPage)
-                var currentLocation = Qt.point(0, listView.contentY - currentItem.y)
+                var currentItem = tableView.itemAtPos(0, tableView.contentY + root.height / 2)
+                var currentPage = currentItem.delegateIndex
+                var currentLocation = Qt.point((tableView.contentX - currentItem.x + root.width / 2) / root.renderScale,
+                                               (tableView.contentY - currentItem.y + root.height / 2) / root.renderScale)
                 if (active) {
                     moved = false
-                    navigationStack.push(currentPage, currentLocation, root.renderScale);
+                    navigationStack.push(currentPage, currentLocation, root.renderScale)
                 } else if (moved) {
-                    navigationStack.update(currentPage, currentLocation, root.renderScale);
+                    navigationStack.update(currentPage, currentLocation, root.renderScale)
                 }
             }
         }
+        ScrollBar.horizontal: ScrollBar { }
+    }
+    onRenderScaleChanged: {
+        tableView.forceLayout()
+        var currentItem = tableView.itemAtPos(tableView.contentX + root.width / 2, tableView.contentY + root.height / 2)
+        if (currentItem !== undefined)
+            navigationStack.update(currentItem.delegateIndex, Qt.point(currentItem.x / renderScale, currentItem.y / renderScale), renderScale)
     }
     PdfNavigationStack {
         id: navigationStack
-        onJumped: listView.currentIndex = page
-        onCurrentPageChanged: {
-            listView.positionViewAtIndex(currentPage, ListView.Beginning)
-            searchModel.currentPage = currentPage
+        onJumped: {
+            root.renderScale = zoom
+            tableView.contentX = Math.max(0, location.x - root.width / 2) * root.renderScale
+            tableView.contentY = tableView.originY + root.document.heightSumBeforePage(page, tableView.rowSpacing / root.renderScale) * root.renderScale
+            if (root.debug) {
+                console.log("going to page", page,
+                            "@y", root.document.heightSumBeforePage(page, tableView.rowSpacing / root.renderScale) * root.renderScale,
+                            "ended up @", tableView.contentY, "originY is", tableView.originY)
+            }
         }
-        onCurrentLocationChanged: listView.contentY += currentLocation.y // currentPageChanged() MUST occur first!
-        onCurrentZoomChanged: root.renderScale = currentZoom
-        // TODO deal with horizontal location (need another Flickable probably)
     }
     PdfSearchModel {
         id: searchModel
diff --git a/src/pdf/quick/qquickpdfdocument.cpp b/src/pdf/quick/qquickpdfdocument.cpp
index 1cfd9a9af..3d5f0fa10 100644
--- a/src/pdf/quick/qquickpdfdocument.cpp
+++ b/src/pdf/quick/qquickpdfdocument.cpp
@@ -90,6 +90,7 @@ void QQuickPdfDocument::setSource(QUrl source)
         return;
 
     m_source = source;
+    m_maxPageWidthHeight = QSizeF();
     emit sourceChanged();
     if (source.scheme() == QLatin1String("qrc"))
         m_doc.load(QLatin1Char(':') + source.path());
@@ -172,6 +173,71 @@ QSizeF QQuickPdfDocument::pagePointSize(int page) const
     return m_doc.pageSize(page);
 }
 
+qreal QQuickPdfDocument::maxPageWidth() const
+{
+    const_cast<QQuickPdfDocument *>(this)->updateMaxPageSize();
+    return m_maxPageWidthHeight.width();
+}
+
+qreal QQuickPdfDocument::maxPageHeight() const
+{
+    const_cast<QQuickPdfDocument *>(this)->updateMaxPageSize();
+    return m_maxPageWidthHeight.height();
+}
+
+/*!
+    \internal
+    \qmlmethod size PdfDocument::heightSumBeforePage(int page)
+
+    Returns the sum of the heights, in points, of all sets of \a facingPages
+    pages from 0 to the given \a page, exclusive.
+
+    That is, if the pages were laid out end-to-end in adjacent sets of
+    \a facingPages, what would be the distance in points from the top of the
+    first page to the top of the given page.
+*/
+// Workaround for lack of something analogous to ListView.positionViewAtIndex() in TableView
+qreal QQuickPdfDocument::heightSumBeforePage(int page, qreal spacing, int facingPages) const
+{
+    qreal ret = 0;
+    for (int i = 0; i < page; i+= facingPages) {
+        if (i + facingPages > page)
+            break;
+        qreal facingPagesHeight = 0;
+        for (int j = i; j < i + facingPages; ++j)
+            facingPagesHeight = qMax(facingPagesHeight, pagePointSize(j).height());
+        ret += facingPagesHeight + spacing;
+    }
+    return ret;
+}
+
+void QQuickPdfDocument::updateMaxPageSize()
+{
+    if (m_maxPageWidthHeight.isValid())
+        return;
+    qreal w = 0;
+    qreal h = 0;
+    const int count = pageCount();
+    for (int i = 0; i < count; ++i) {
+        auto size = pagePointSize(i);
+        w = qMax(w, size.width());
+        h = qMax(w, size.height());
+    }
+    m_maxPageWidthHeight = QSizeF(w, h);
+}
+
+/*!
+    \qmlproperty real PdfDocument::maxPageWidth
+
+    This property holds the width of the widest page in the document, in points.
+*/
+
+/*!
+    \qmlproperty real PdfDocument::maxPageHeight
+
+    This property holds the height of the tallest page in the document, in points.
+*/
+
 /*!
     \qmlproperty string PdfDocument::title
 
diff --git a/src/pdf/quick/qquickpdfdocument_p.h b/src/pdf/quick/qquickpdfdocument_p.h
index 9817b5eef..cefa4f756 100644
--- a/src/pdf/quick/qquickpdfdocument_p.h
+++ b/src/pdf/quick/qquickpdfdocument_p.h
@@ -62,6 +62,8 @@ class QQuickPdfDocument : public QObject, public QQmlParserStatus
     Q_OBJECT
     Q_PROPERTY(QUrl source READ source WRITE setSource NOTIFY sourceChanged)
     Q_PROPERTY(int pageCount READ pageCount NOTIFY pageCountChanged FINAL)
+    Q_PROPERTY(qreal maxPageWidth READ maxPageWidth NOTIFY metaDataChanged)
+    Q_PROPERTY(qreal maxPageHeight READ maxPageHeight NOTIFY metaDataChanged)
     Q_PROPERTY(QString password READ password WRITE setPassword NOTIFY passwordChanged FINAL)
     Q_PROPERTY(QPdfDocument::Status status READ status NOTIFY statusChanged FINAL)
     Q_PROPERTY(QString error READ error NOTIFY statusChanged FINAL)
@@ -102,6 +104,9 @@ public:
     QDateTime modificationDate() { return m_doc.metaData(QPdfDocument::ModificationDate).toDateTime(); }
 
     Q_INVOKABLE QSizeF pagePointSize(int page) const;
+    qreal maxPageWidth() const;
+    qreal maxPageHeight() const;
+    Q_INVOKABLE qreal heightSumBeforePage(int page, qreal spacing = 0, int facingPages = 1) const;
 
 Q_SIGNALS:
     void sourceChanged();
@@ -113,10 +118,12 @@ Q_SIGNALS:
 
 private:
     QPdfDocument &document() { return m_doc; }
+    void updateMaxPageSize();
 
 private:
     QUrl m_source;
     QPdfDocument m_doc;
+    QSizeF m_maxPageWidthHeight;
 
     friend class QQuickPdfLinkModel;
     friend class QQuickPdfSearchModel;
diff --git a/src/pdf/quick/qquickpdfnavigationstack.cpp b/src/pdf/quick/qquickpdfnavigationstack.cpp
index 51f65f032..7ba317557 100644
--- a/src/pdf/quick/qquickpdfnavigationstack.cpp
+++ b/src/pdf/quick/qquickpdfnavigationstack.cpp
@@ -80,11 +80,11 @@ void QQuickPdfNavigationStack::forward()
     ++m_currentHistoryIndex;
     m_changing = true;
     emit jumped(currentPage(), currentLocation(), currentZoom());
+    if (currentZoomWas != currentZoom())
+        emit currentZoomChanged();
     emit currentPageChanged();
     if (currentLocationWas != currentLocation())
         emit currentLocationChanged();
-    if (currentZoomWas != currentZoom())
-        emit currentZoomChanged();
     if (!backAvailableWas)
         emit backAvailableChanged();
     if (forwardAvailableWas != forwardAvailable())
@@ -110,11 +110,11 @@ void QQuickPdfNavigationStack::back()
     --m_currentHistoryIndex;
     m_changing = true;
     emit jumped(currentPage(), currentLocation(), currentZoom());
+    if (currentZoomWas != currentZoom())
+        emit currentZoomChanged();
     emit currentPageChanged();
     if (currentLocationWas != currentLocation())
         emit currentLocationChanged();
-    if (currentZoomWas != currentZoom())
-        emit currentZoomChanged();
     if (backAvailableWas != backAvailable())
         emit backAvailableChanged();
     if (!forwardAvailableWas)
@@ -183,15 +183,16 @@ void QQuickPdfNavigationStack::push(int page, QPointF location, qreal zoom)
         m_pageHistory.append(QExplicitlySharedDataPointer<QPdfDestinationPrivate>(new QPdfDestinationPrivate(page, location, zoom)));
         m_currentHistoryIndex = m_pageHistory.count() - 1;
     }
+    emit currentZoomChanged();
     emit currentPageChanged();
     emit currentLocationChanged();
-    emit currentZoomChanged();
     if (m_changing)
         return;
     if (!backAvailableWas)
         emit backAvailableChanged();
     if (forwardAvailableWas)
         emit forwardAvailableChanged();
+    emit jumped(page, location, zoom);
     qCDebug(qLcNav) << "push: index" << m_currentHistoryIndex << "page" << page
                     << "@" << location << "zoom" << zoom << "-> history" <<
         [this]() {
@@ -212,7 +213,7 @@ void QQuickPdfNavigationStack::push(int page, QPointF location, qreal zoom)
     the most-recently-viewed destination rather than the destination that was
     last specified by push().
 
-    The \c currentPageChanged, \c currentLocationChanged and \c currentZoomChanged
+    The \c currentZoomChanged, \c currentPageChanged and \c currentLocationChanged
     signals will be emitted if the respective properties are actually changed.
     The \l jumped signal is not emitted, because this operation
     represents smooth movement rather than a navigational jump.
@@ -229,12 +230,12 @@ void QQuickPdfNavigationStack::update(int page, QPointF location, qreal zoom)
     m_pageHistory[m_currentHistoryIndex]->page = page;
     m_pageHistory[m_currentHistoryIndex]->location = location;
     m_pageHistory[m_currentHistoryIndex]->zoom = zoom;
+    if (currentZoomWas != zoom)
+        emit currentZoomChanged();
     if (currentPageWas != page)
         emit currentPageChanged();
     if (currentLocationWas != location)
         emit currentLocationChanged();
-    if (currentZoomWas != zoom)
-        emit currentZoomChanged();
     qCDebug(qLcNav) << "update: index" << m_currentHistoryIndex << "page" << page
                     << "@" << location << "zoom" << zoom << "-> history" <<
         [this]() {
@@ -258,10 +259,8 @@ bool QQuickPdfNavigationStack::forwardAvailable() const
 /*!
     \qmlsignal PdfNavigationStack::jumped(int page, point location, qreal zoom)
 
-    This signal is emitted when either forward() or back() is called, to
-    distinguish navigational jumps from cases when push() is called.
-    Contrast with the \c currentPageChanged signal, which is emitted in all
-    cases, and does not include the \c page, \c location and \c zoom arguments.
+    This signal is emitted when forward(), back() or push() is called, but not
+    when update() is called.
 */
 
 QT_END_NAMESPACE