fix: apply the adversarial-review findings for the dev train promoted by #272

.github/workflows/ci.yml

@@ -130,6 +130,11 @@ jobs:
         run: |
           python3 scripts/ci/test_command_catalog.py
           python3 scripts/verify-command-catalog.py
+      - name: Verify Quick localization and operator help strings (#126)
+        run: |
+          python3 -m unittest scripts.ci.test_check_quick_localization -q
+          python3 scripts/update-quick-help-strings.py --check
+          python3 scripts/ci/check_quick_localization.py
       - name: Verify preflight check catalog and coverage backlog (#165)
         run: python3 scripts/ci/test_preflight_check_catalog.py
       - name: Verify preflight corpus coverage map (#165)

LoopEditor/editorhost.cpp

@@ -73,6 +73,7 @@
 #include <QClipboard>
 #include <QCoreApplication>
 #include <QCryptographicHash>
+#include <QDesktopServices>
 #include <QDir>
 #include <QFile>
 #include <QFileInfo>
@@ -86,6 +87,7 @@
 #include <QStandardPaths>
 #include <QUuid>
 #include <QUrl>
+#include <QUrlQuery>
 
 #include <algorithm>
 #include <limits>
@@ -666,6 +668,15 @@ bool EditorHost::copySelectedText()
         return false;
     }
 
+    // The document's own copy permission decides before the controller is
+    // asked, and the refusal carries the sentence the CLI refuses with, so a
+    // copy-restricted document never reaches the clipboard through this seam.
+    if (!m_documentModel.canCopy())
+    {
+        announceDocumentState(tr("Copy refused: %1").arg(tr("Document doesn't allow to copy content.")));
+        return false;
+    }
+
     // The refusal is typed and named, and the clipboard is left untouched: an
     // empty string is never copied as success.
     const pdfinteraction::TextCopyResult result = interaction->selectedText();
@@ -2893,16 +2904,27 @@ void EditorHost::finishInkCoverageJob(const pdf::PDFJobSnapshot& snapshot)
         return;
     }
 
-    if (!outcome->ok)
+    const bool measured = snapshot.status == pdf::PDFJobStatus::Succeeded;
+    if (!measured || !outcome->ok)
     {
-        // A readout that could not be produced is a visible state, never a zero:
-        // "0% ink" and "not measured" are different facts.
+        // A readout that could not be produced, or one the scheduler has since
+        // superseded, cancelled or failed, is a visible state, never a zero:
+        // "0% ink" and "not measured" are different facts, and ink numbers
+        // measured against another revision are not this revision's numbers.
+        QString reason = outcome->reason;
+        if (!measured)
+        {
+            reason = QStringLiteral("ink-coverage/stale-or-cancelled");
+        }
+        else if (reason.isEmpty())
+        {
+            reason = QStringLiteral("ink-coverage/not-measured");
+        }
+
         QVariantMap readout;
         readout.insert(QStringLiteral("available"), false);
         readout.insert(QStringLiteral("pageIndex"), outcome->pageIndex);
-        readout.insert(QStringLiteral("reason"), outcome->reason.isEmpty()
-                                                     ? QStringLiteral("ink-coverage/not-measured")
-                                                     : outcome->reason);
+        readout.insert(QStringLiteral("reason"), reason);
         readout.insert(QStringLiteral("channels"), QVariantList());
         m_inkCoverageReadout = readout;
     }
@@ -4887,6 +4909,10 @@ void EditorHost::connectFacade()
             [this](const pdf::PDFRevisionIdentity&, const pdf::PDFRevisionIdentity&)
             {
                 cancelPreflight();
+                // The ink-coverage readout measured the revision that just left;
+                // dropping it is the honest state, and a fresh measurement
+                // repopulates it.
+                m_inkCoverageReadout = QVariantMap();
                 syncRevisionModels();
                 if (m_documentBound)
                 {
@@ -5578,6 +5604,17 @@ pdfinteraction::DirectEditOutcome EditorHost::submitDocumentDirectEdit(pdfintera
     return m_session->facade().applyDirectEdit(request);
 }
 
+QUrl EditorHost::mailHandoffUrl(const QString& path, const QString& revision)
+{
+    const QString name = QFileInfo(path).fileName();
+    QUrlQuery query;
+    query.addQueryItem(QStringLiteral("subject"), tr("Published document: %1").arg(name));
+    query.addQueryItem(QStringLiteral("body"), tr("The published document %1 is at revision %2.").arg(path, revision));
+    QUrl url(QStringLiteral("mailto:"));
+    url.setQuery(query);
+    return url;
+}
+
 bool EditorHost::handOffPublishedRevisionToMail(QString* typedError)
 {
     typedError->clear();
@@ -5603,9 +5640,16 @@ bool EditorHost::handOffPublishedRevisionToMail(QString* typedError)
         return false;
     }
 
-    // The revision travels with the hand-off, so the receiving side can refuse a
-    // file that is not the revision the shell was showing.
-    Q_EMIT documentMailRequested(source.path, facade.currentRevision().toString());
+    // The revision travels in the message body, so the recipient can tell which
+    // revision of the file they hold. A client that does not take the message is
+    // a refusal, not a hand-off.
+    const QString revision = facade.currentRevision().toString();
+    if (!QDesktopServices::openUrl(mailHandoffUrl(source.path, revision)))
+    {
+        *typedError = QStringLiteral("mail/client-unavailable");
+        return false;
+    }
+    Q_EMIT documentMailRequested(source.path, revision);
     announceDocumentState(tr("The published document was handed to the mail client."));
     return true;
 }
@@ -5676,17 +5720,19 @@ void EditorHost::refreshFeatureAvailability()
     availability.insert(MoveSelectionCommandId, ready && m_session->facade().permissions().allowsCorrection());
 
     // Select, copy and capture (issue #232). Every one of these is enabled with
-    // a document: Copy deliberately stays enabled without a selection so its
-    // typed refusal is reachable and visible, rather than the command silently
-    // disappearing from the menu.
+    // a document, and copy and extraction additionally follow the document's
+    // copy permission: Copy deliberately stays enabled without a selection so
+    // its typed refusal is reachable and visible, rather than the command
+    // silently disappearing from the menu.
     for (const QString& id : { QStringLiteral("actionSelectText"), QStringLiteral("actionSelectTextAll"),
                                QStringLiteral("actionDeselectText"), QStringLiteral("actionSelectTable"),
-                               QStringLiteral("actionCopyText"), QStringLiteral("actionGoToNextLine"),
-                               QStringLiteral("actionGoToPreviousLine"), QStringLiteral("actionScreenshot"),
-                               QStringLiteral("actionExtractImage") })
+                               QStringLiteral("actionGoToNextLine"), QStringLiteral("actionGoToPreviousLine"),
+                               QStringLiteral("actionScreenshot") })
     {
         availability.insert(id, ready);
     }
+    availability.insert(QStringLiteral("actionCopyText"), ready && m_documentModel.canCopy());
+    availability.insert(QStringLiteral("actionExtractImage"), ready && m_documentModel.canCopy());
 
     // The developer-diagnostics shell surfaces (#239): settings, render options
     // and the diagnostics bundle. They follow the same flag the menu gate

LoopEditor/editorhost.h

@@ -724,10 +724,10 @@ class EditorHost final : public QObject
     /// asks for one and answers with extractCurrentPageImagesFileUrl().
     void imageExtractionRequested();
 
-    /// Send by e-mail hands the current published revision to the shell's mail
+    /// Send by e-mail hands the current published revision to the desktop mail
     /// client. `path` is the published artifact and `revision` is the identity it
-    /// was published as, so the receiving side can refuse a mismatch. Emitted
-    /// only for an open document with no unsaved changes: an unsaved or stale
+    /// was published as. Emitted only after the mail client accepted the message,
+    /// and only for an open document with no unsaved changes: an unsaved or stale
     /// revision is refused instead, never mailed as if it were the file.
     void documentMailRequested(const QString& path, const QString& revision);
 
@@ -763,8 +763,10 @@ class EditorHost final : public QObject
     /// Hands the current published revision to the shell's mail client. Refuses
     /// when there is no open document, when its source is not a file, or when it
     /// has unsaved changes: the bytes on disk would then be a different revision
-    /// than the one on screen. Never writes and never opens anything itself.
+    /// than the one on screen. Never writes; the mail client is opened only once
+    /// every refusal above has passed.
     bool handOffPublishedRevisionToMail(QString* typedError);
+    static QUrl mailHandoffUrl(const QString& path, const QString& revision);
     bool requestMoveSelection(const QVariantMap& parameters);
 
     // ---- Bookmarks and hyperlinks (issue #233) -------------------------------------

LoopEditor/qml/DocumentPane.qml

@@ -84,6 +84,15 @@ Item {
                     TabButton {
                         objectName: "annotationsTabButton"
                         text: qsTr("Annotations")
+                        contentItem: Text {
+                            text: parent.text
+                            font: parent.font
+                            color: parent.palette.buttonText
+                            wrapMode: Text.WordWrap
+                            horizontalAlignment: Text.AlignHCenter
+                            verticalAlignment: Text.AlignVCenter
+                            Accessible.ignored: true
+                        }
                         Accessible.name: qsTr("Annotations tab")
                     }
                 }

LoopEditor/qml/Main.qml

@@ -338,6 +338,35 @@ ApplicationWindow {
         onRejected: if (host && host.focusRestoration) host.focusRestoration.restore()
     }
 
+    FileDialog {
+        id: bookmarkImportDialog
+        title: qsTr("Import Bookmarks")
+        nameFilters: [qsTr("Bookmark files (*.json)")]
+        property string revision: ""
+        onAccepted: {
+            if (host) {
+                host.importBookmarksFileUrl(selectedFile, revision)
+            }
+            if (host && host.focusRestoration) host.focusRestoration.restore()
+        }
+        onRejected: if (host && host.focusRestoration) host.focusRestoration.restore()
+    }
+
+    FileDialog {
+        id: bookmarkExportDialog
+        title: qsTr("Export Bookmarks")
+        fileMode: FileDialog.SaveFile
+        nameFilters: [qsTr("Bookmark files (*.json)")]
+        property string revision: ""
+        onAccepted: {
+            if (host) {
+                host.exportBookmarksFileUrl(selectedFile, revision)
+            }
+            if (host && host.focusRestoration) host.focusRestoration.restore()
+        }
+        onRejected: if (host && host.focusRestoration) host.focusRestoration.restore()
+    }
+
     FileDialog {
         id: preflightReportDialog
         title: qsTr("Export Preflight Report")
@@ -414,6 +443,16 @@ ApplicationWindow {
         function onImageExtractionRequested() {
             extractImagesDialog.open()
         }
+        function onBookmarkImportRequested() {
+            if (host && host.focusRestoration) host.focusRestoration.remember(window.activeFocusItem)
+            bookmarkImportDialog.revision = host.outputRevision()
+            bookmarkImportDialog.open()
+        }
+        function onBookmarkExportRequested() {
+            if (host && host.focusRestoration) host.focusRestoration.remember(window.activeFocusItem)
+            bookmarkExportDialog.revision = host.outputRevision()
+            bookmarkExportDialog.open()
+        }
         function onSettingsPanelRequested() {
             if (host && host.focusRestoration) host.focusRestoration.remember(window.activeFocusItem)
             settingsDialog.open()

LoopEditor/qml/MenuModel.qml

@@ -127,6 +127,7 @@ QtObject {
         case "actionDeselectText": return qsTr("Deselect Text")
         case "actionEditDocumentMetadata": return qsTr("Edit Document Metadata")
         case "actionEncryption": return qsTr("Encryption")
+        case "actionExportDocument": return qsTr("Export document…")
         case "actionExtractImage": return qsTr("Extract Image")
         case "actionFind": return qsTr("Find")
         case "actionFindNext": return qsTr("Find Next")

LoopEditor/qml/ProductionPreviewPane.qml

@@ -148,7 +148,7 @@ Pane {
                         ? ""
                         : qsTr("Soft proof %1: %2")
                             .arg(root.host.softProofStateName)
-                            .arg(root.host.softProofSummary())
+                            .arg(root.host.softProofSummary)
                     Accessible.name: qsTr("Soft proof state")
                     Accessible.description: text
                 }

LoopLibCore/sources/pdfauthoringedit.cpp

@@ -73,9 +73,16 @@ PDFObjectReference pageReference(const PDFCatalog* catalog, int pageIndex)
 }
 
 /// Resolves a destination's page. A destination may name the page by reference
-/// or by index; a reference that resolves to no page is not a page target.
+/// or by index; a named or unparsed destination has no page target here
+/// (callers refuse it with their own typed code), and a reference that
+/// resolves to no page is not a page target.
 bool resolveDestinationPage(const PDFCatalog* catalog, const PDFDestination& destination, int* pageIndex)
 {
+    if (!destination.isValid() || destination.isNamedDestination())
+    {
+        return false;
+    }
+
     if (destination.getPageReference().isValid())
     {
         const size_t resolved = catalog->getPageIndexFromPageReference(destination.getPageReference());
@@ -157,8 +164,9 @@ bool removePageBookmarks(const PDFCatalog* catalog, PDFOutlineItem* item, int pa
 }
 
 /// True when the builder's outline writer can write this item back unchanged.
-/// It writes Title, a GoTo Dest, a GoToDp action, colour and flags; a
-/// structure element or any other action would be silently dropped.
+/// It writes Title, a GoTo Dest, a GoToDp action, colour and flags; a structure
+/// element, a GoTo whose destination did not parse (the writer would write PDF
+/// null there) or any other action would be silently dropped.
 bool isRewritableItem(const PDFOutlineItem* item)
 {
     if (item->getStructureElement().isValid())
@@ -173,8 +181,15 @@ bool isRewritableItem(const PDFOutlineItem* item)
         return true;
     }
 
-    return dynamic_cast<const PDFActionGoTo*>(action) != nullptr ||
-           dynamic_cast<const PDFActionGoToDp*>(action) != nullptr;
+    const auto* goTo = dynamic_cast<const PDFActionGoTo*>(action);
+    if (goTo)
+    {
+        // A named destination is valid and round-trips (the object factory
+        // writes the name); only a destination that failed to parse is lossy.
+        return goTo->getDestination().isValid();
+    }
+
+    return dynamic_cast<const PDFActionGoToDp*>(action) != nullptr;
 }
 
 bool canRewriteOutline(const PDFOutlineItem* item)