From 44c86de7e415e30572d774a3ec3e90681bc8abfa Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Mon, 3 Jan 2022 16:01:01 +0100 Subject: [PATCH] Fix two Signature issues on multipage/multisignature documents First issue: - The "document is totally signed" was based on the last signature of the last page (that had signatures) that is not correct and needs to be based in the last signature by date - The "Rev #" number was based on the signature on the page, so if we had two pages with one signature each the model showed "Rev 1" for both It adds new API which is not awesome in a stable branch, but the first issue is important enough that warrants this to go to the stable branch --- core/form.cpp | 6 +++ core/form.h | 7 ++++ core/form_p.h | 1 + core/page.cpp | 1 + part/part.cpp | 2 +- part/signatureguiutils.cpp | 11 ++++-- part/signatureguiutils.h | 5 +-- part/signaturemodel.cpp | 59 ++++++++++++++++-------------- part/signaturepanel.cpp | 2 +- part/signaturepropertiesdialog.cpp | 2 +- 10 files changed, 59 insertions(+), 37 deletions(-) diff --git a/core/form.cpp b/core/form.cpp index 0cfb80dfe..bc5f599ca 100644 --- a/core/form.cpp +++ b/core/form.cpp @@ -115,6 +115,12 @@ void FormField::setAdditionalAction(Annotation::AdditionalActionType type, Actio d->m_additionalAnnotActions[type] = action; } +Page *FormField::page() const +{ + Q_D(const FormField); + return d->m_page; +} + class Okular::FormFieldButtonPrivate : public Okular::FormFieldPrivate { public: diff --git a/core/form.h b/core/form.h index 477092fbf..4ac71974a 100644 --- a/core/form.h +++ b/core/form.h @@ -157,6 +157,13 @@ public: */ Action *additionalAction(Annotation::AdditionalActionType type) const; + /** + * Returns the page of this form field + * + * @since 21.12.2 + */ + Page *page() const; + protected: /// @cond PRIVATE explicit FormField(FormFieldPrivate &dd); diff --git a/core/form_p.h b/core/form_p.h index 4bedfe2e7..d4a6ca46e 100644 --- a/core/form_p.h +++ b/core/form_p.h @@ -35,6 +35,7 @@ public: Action *m_activateAction; QHash m_additionalActions; QHash m_additionalAnnotActions; + Page *m_page = nullptr; Q_DECLARE_PUBLIC(FormField) FormField *q_ptr; diff --git a/core/page.cpp b/core/page.cpp index aaab912cd..94dc03e65 100644 --- a/core/page.cpp +++ b/core/page.cpp @@ -712,6 +712,7 @@ void Page::setFormFields(const QLinkedList &fields) d->formfields = fields; for (FormField *ff : qAsConst(d->formfields)) { ff->d_ptr->setDefault(); + ff->d_ptr->m_page = this; } } diff --git a/part/part.cpp b/part/part.cpp index b55f3c94e..0f4a64493 100644 --- a/part/part.cpp +++ b/part/part.cpp @@ -1586,7 +1586,7 @@ bool Part::openFile() if (m_embedMode == PrintPreviewMode) { m_signatureMessage->setText(i18n("All editing and interactive features for this document are disabled. Please save a copy and reopen to edit this document.")); } else { - const QVector signatureFormFields = SignatureGuiUtils::getSignatureFormFields(m_document, true, 0); + const QVector signatureFormFields = SignatureGuiUtils::getSignatureFormFields(m_document); bool allSignaturesValid = true; for (const Okular::FormFieldSignature *signature : signatureFormFields) { const Okular::SignatureInfo &info = signature->signatureInfo(); diff --git a/part/signatureguiutils.cpp b/part/signatureguiutils.cpp index 0adc71cbd..e497038ac 100644 --- a/part/signatureguiutils.cpp +++ b/part/signatureguiutils.cpp @@ -14,10 +14,10 @@ namespace SignatureGuiUtils { -QVector getSignatureFormFields(Okular::Document *doc, bool allPages, int pageNum) +QVector getSignatureFormFields(Okular::Document *doc) { - uint curPage = allPages ? 0 : pageNum; - const uint endPage = allPages ? doc->pages() - 1 : pageNum; + uint curPage = 0; + const uint endPage = doc->pages() - 1; QVector signatureFormFields; while (curPage <= endPage) { const QLinkedList formFields = doc->page(curPage++)->formFields(); @@ -27,6 +27,11 @@ QVector getSignatureFormFields(Okular::Docum } } } + std::sort(signatureFormFields.begin(), signatureFormFields.end(), [](const Okular::FormFieldSignature *a, const Okular::FormFieldSignature *b) { + const Okular::SignatureInfo &infoA = a->signatureInfo(); + const Okular::SignatureInfo &infoB = b->signatureInfo(); + return infoA.signingTime() < infoB.signingTime(); + }); return signatureFormFields; } diff --git a/part/signatureguiutils.h b/part/signatureguiutils.h index 4af897167..eba06a5db 100644 --- a/part/signatureguiutils.h +++ b/part/signatureguiutils.h @@ -20,10 +20,9 @@ class FormFieldSignature; namespace SignatureGuiUtils { /** - * Returns a vector containing signature form fields. If @p allPages is true then all signature form fields in the - * document are returned otherwise the fields in page number @p pageNum are returned. + * Returns a vector containing signature form fields sorted by date (last is newer). */ -QVector getSignatureFormFields(Okular::Document *doc, bool allPages, int pageNum); +QVector getSignatureFormFields(Okular::Document *doc); QString getReadableSignatureStatus(Okular::SignatureInfo::SignatureStatus sigStatus); QString getReadableCertStatus(Okular::SignatureInfo::CertificateStatus certStatus); QString getReadableHashAlgorithm(Okular::SignatureInfo::HashAlgorithm hashAlg); diff --git a/part/signaturemodel.cpp b/part/signaturemodel.cpp index c92d52729..324b2c7b7 100644 --- a/part/signaturemodel.cpp +++ b/part/signaturemodel.cpp @@ -117,36 +117,39 @@ void SignatureModelPrivate::notifySetup(const QVector &pages, in q->beginResetModel(); qDeleteAll(root->children); root->children.clear(); - for (const Okular::Page *page : pages) { - const int currentPage = page->number(); - // get form fields page by page so that page number and index of the form can be determined. - const QVector signatureFormFields = SignatureGuiUtils::getSignatureFormFields(document, false, currentPage); - if (signatureFormFields.isEmpty()) - continue; - - for (int i = 0; i < signatureFormFields.count(); i++) { - const Okular::FormFieldSignature *sf = signatureFormFields[i]; - const Okular::SignatureInfo &info = sf->signatureInfo(); - - // based on whether or not signature form is a nullptr it is decided if clicking on an item should change the viewport. - auto *parentItem = new SignatureItem(root, sf, SignatureItem::RevisionInfo, currentPage); - parentItem->displayString = i18n("Rev. %1: Signed By %2", i + 1, info.signerName()); - - auto childItem1 = new SignatureItem(parentItem, nullptr, SignatureItem::ValidityStatus, currentPage); - childItem1->displayString = SignatureGuiUtils::getReadableSignatureStatus(info.signatureStatus()); - - auto childItem2 = new SignatureItem(parentItem, nullptr, SignatureItem::SigningTime, currentPage); - childItem2->displayString = i18n("Signing Time: %1", info.signingTime().toString(Qt::DefaultLocaleLongDate)); - - const QString reason = info.reason(); - if (!reason.isEmpty()) { - auto childItem3 = new SignatureItem(parentItem, nullptr, SignatureItem::Reason, currentPage); - childItem3->displayString = i18n("Reason: %1", reason); - } - auto childItem4 = new SignatureItem(parentItem, sf, SignatureItem::FieldInfo, currentPage); - childItem4->displayString = i18n("Field: %1 on page %2", sf->name(), currentPage + 1); + if (pages.isEmpty()) { + q->endResetModel(); + return; + } + + int revNumber = 1; + const QVector signatureFormFields = SignatureGuiUtils::getSignatureFormFields(document); + for (const Okular::FormFieldSignature *sf : signatureFormFields) { + const Okular::SignatureInfo &info = sf->signatureInfo(); + + const int pageNumber = sf->page()->number(); + + // based on whether or not signature form is a nullptr it is decided if clicking on an item should change the viewport. + auto *parentItem = new SignatureItem(root, sf, SignatureItem::RevisionInfo, pageNumber); + parentItem->displayString = i18n("Rev. %1: Signed By %2", revNumber, info.signerName()); + + auto childItem1 = new SignatureItem(parentItem, nullptr, SignatureItem::ValidityStatus, pageNumber); + childItem1->displayString = SignatureGuiUtils::getReadableSignatureStatus(info.signatureStatus()); + + auto childItem2 = new SignatureItem(parentItem, nullptr, SignatureItem::SigningTime, pageNumber); + childItem2->displayString = i18n("Signing Time: %1", info.signingTime().toString(Qt::DefaultLocaleLongDate)); + + const QString reason = info.reason(); + if (!reason.isEmpty()) { + auto childItem3 = new SignatureItem(parentItem, nullptr, SignatureItem::Reason, pageNumber); + childItem3->displayString = i18n("Reason: %1", reason); } + + auto childItem4 = new SignatureItem(parentItem, sf, SignatureItem::FieldInfo, pageNumber); + childItem4->displayString = i18n("Field: %1 on page %2", sf->name(), pageNumber + 1); + + ++revNumber; } q->endResetModel(); } diff --git a/part/signaturepanel.cpp b/part/signaturepanel.cpp index 722af6504..06a779d74 100644 --- a/part/signaturepanel.cpp +++ b/part/signaturepanel.cpp @@ -111,7 +111,7 @@ void SignaturePanel::notifySetup(const QVector & /*pages*/, int return; Q_D(SignaturePanel); - const QVector signatureForms = SignatureGuiUtils::getSignatureFormFields(d->m_document, true, -1); + const QVector signatureForms = SignatureGuiUtils::getSignatureFormFields(d->m_document); emit documentHasSignatures(!signatureForms.isEmpty()); } diff --git a/part/signaturepropertiesdialog.cpp b/part/signaturepropertiesdialog.cpp index 4604c570a..8c379416c 100644 --- a/part/signaturepropertiesdialog.cpp +++ b/part/signaturepropertiesdialog.cpp @@ -88,7 +88,7 @@ SignaturePropertiesDialog::SignaturePropertiesDialog(Okular::Document *doc, cons if (signatureStatus != Okular::SignatureInfo::SignatureStatusUnknown && !signatureInfo.signsTotalDocument()) { revisionBox = new QGroupBox(i18n("Document Version")); auto revisionLayout = new QHBoxLayout(revisionBox); - const QVector signatureFormFields = SignatureGuiUtils::getSignatureFormFields(m_doc, true, -1); + const QVector signatureFormFields = SignatureGuiUtils::getSignatureFormFields(m_doc); revisionLayout->addWidget(new QLabel(i18nc("Document Revision of ", "Document Revision %1 of %2", signatureFormFields.indexOf(m_signatureForm) + 1, signatureFormFields.size()))); revisionLayout->addStretch(); auto revisionBtn = new QPushButton(i18n("View Signed Version..."));