cool#10630 doc sign: fix Impress sign line, to be able to finish signing again

Open a PDF for signing on the desktop, insert a signature line, click
the "finish signing" button on the infobar, Draw first asks if you want
to save your modified document as ODG, then crashes.

The first problem is that opening a PDF for signing and adding a
signature line / visual signature didn't result in a modified doc model,
but that now happens since commit
aca32a55456aa4e907b216fb490b3c15d26c3d55 (tdf#146547 sfx2: allow
read-only documents to be modified, 2023-06-16). Probably we want to
keep this feaature that read-only documents can be modified in some
cases, so explicitly mark the PDF document as not-modified before
signing in SfxObjectShell::SignDocumentContentUsingCertificate().

The other problem is that visual signing tried to reload the document:
this is handy, as it shows if our PDF export worked correctly, but
reloading is problematic since the signing code moved to async and it
assumes that the underlying object shell is never deleted, so callbacks
can have a pointer to it without worrying about the lifecycle. Fix this
problem by not reloading, but clearing the marker on the signature shape
and invalidating the signature status, instead of reloading, in
SfxObjectShell::ExecFile_Impl().

With this, visual signing of an existing PDF file works again in desktop
Draw.

Change-Id: I743983947d4607aa06674a1329bc8efb5af8a6d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/179856
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

2 files changed, 49 insertions, 27 deletions

diff --git a/include/sfx2/objsh.hxx b/include/sfx2/objsh.hxx
index de6b46e23ea8..3238e40d85a9 100644
--- a/include/sfx2/objsh.hxx
+++ b/include/sfx2/objsh.hxx
@@ -814,6 +814,8 @@ public:
     /// Gets the certificate that is already picked by the user but not yet used for signing.
     css::uno::Reference<css::security::XCertificate> GetSignPDFCertificate() const;
 
+    void ResetSignPDFCertificate();
+
     /// Gets grab-bagged password info to unprotect change tracking with verification
     css::uno::Sequence< css::beans::PropertyValue > GetDocumentProtectionFromGrabBag() const;
 
diff --git a/sfx2/source/doc/objserv.cxx b/sfx2/source/doc/objserv.cxx
index 25a541691921..674afcbad1d2 100644
--- a/sfx2/source/doc/objserv.cxx
+++ b/sfx2/source/doc/objserv.cxx
@@ -430,27 +430,30 @@ bool SfxObjectShell::IsSignPDF() const
     return false;
 }
 
-uno::Reference<security::XCertificate> SfxObjectShell::GetSignPDFCertificate() const
+namespace
+{
+uno::Reference<beans::XPropertySet> GetSelectedShapeOfModel(const uno::Reference<frame::XModel>& xModel)
 {
-    uno::Reference<frame::XModel> xModel = GetBaseModel();
     if (!xModel.is())
     {
-        return uno::Reference<security::XCertificate>();
+        return uno::Reference<beans::XPropertySet>();
     }
 
     uno::Reference<drawing::XShapes> xShapes(xModel->getCurrentSelection(), uno::UNO_QUERY);
     if (!xShapes.is() || xShapes->getCount() < 1)
     {
-        return uno::Reference<security::XCertificate>();
+        return uno::Reference<beans::XPropertySet>();
     }
 
     uno::Reference<beans::XPropertySet> xShapeProps(xShapes->getByIndex(0), uno::UNO_QUERY);
-    if (!xShapeProps.is())
-    {
-        return uno::Reference<security::XCertificate>();
-    }
+    return xShapeProps;
+}
+}
 
-    if (!xShapeProps->getPropertySetInfo()->hasPropertyByName(u"InteropGrabBag"_ustr))
+uno::Reference<security::XCertificate> SfxObjectShell::GetSignPDFCertificate() const
+{
+    uno::Reference<beans::XPropertySet> xShapeProps = GetSelectedShapeOfModel(GetBaseModel());
+    if (!xShapeProps.is() || !xShapeProps->getPropertySetInfo()->hasPropertyByName(u"InteropGrabBag"_ustr))
     {
         return uno::Reference<security::XCertificate>();
     }
@@ -465,6 +468,27 @@ uno::Reference<security::XCertificate> SfxObjectShell::GetSignPDFCertificate() c
     return uno::Reference<security::XCertificate>(it->second, uno::UNO_QUERY);
 }
 
+void SfxObjectShell::ResetSignPDFCertificate()
+{
+    uno::Reference<beans::XPropertySet> xShapeProps = GetSelectedShapeOfModel(GetBaseModel());
+    if (!xShapeProps->getPropertySetInfo()->hasPropertyByName("InteropGrabBag"))
+    {
+        return;
+    }
+
+    comphelper::SequenceAsHashMap aMap(xShapeProps->getPropertyValue("InteropGrabBag"));
+    auto it = aMap.find("SignatureCertificate");
+    if (it == aMap.end())
+    {
+        return;
+    }
+
+    aMap.erase(it);
+    xShapeProps->setPropertyValue("InteropGrabBag", uno::Any(aMap.getAsConstPropertyValueList()));
+    // The shape's property is now reset, so the doc model is no longer modified.
+    SetModified(false);
+}
+
 static void sendErrorToLOK(const ErrCodeMsg& error)
 {
     if (error.GetCode().GetClass() == ErrCodeClass::NONE)
@@ -585,26 +609,15 @@ void SfxObjectShell::ExecFile_Impl(SfxRequest &rReq)
                 aSigningContext.m_xCertificate = std::move(xCertificate);
                 bHaveWeSigned |= SignDocumentContentUsingCertificate(aSigningContext);
 
-                // Reload to show how the PDF actually looks like after signing. This also
-                // changes "finish signing" on the infobar back to "sign document" as a side
-                // effect.
+                // Reset the picked certificate for PDF signing, then recheck signatures to show how
+                // the PDF actually looks like after signing.  Also change the "finish signing" on
+                // the infobar back to "sign document".
                 if (SfxViewFrame* pFrame = GetFrame())
                 {
-                    // Store current page before reload.
-                    SfxAllItemSet aSet(SfxGetpApp()->GetPool());
-                    uno::Reference<drawing::XDrawView> xController(
-                        GetBaseModel()->getCurrentController(), uno::UNO_QUERY);
-                    uno::Reference<beans::XPropertySet> xPage(xController->getCurrentPage(),
-                                                              uno::UNO_QUERY);
-                    sal_Int32 nPage{};
-                    xPage->getPropertyValue(u"Number"_ustr) >>= nPage;
-                    if (nPage > 0)
-                    {
-                        // nPage is 1-based.
-                        aSet.Put(SfxInt32Item(SID_PAGE_NUMBER, nPage - 1));
-                    }
-                    SfxRequest aReq(SID_RELOAD, SfxCallMode::SLOT, aSet);
-                    pFrame->ExecReload_Impl(aReq);
+                    ResetSignPDFCertificate();
+                    RecheckSignature(false);
+                    pFrame->RemoveInfoBar(u"readonly");
+                    pFrame->AppendReadOnlyInfobar();
                 }
             }
             else
@@ -2249,6 +2262,13 @@ bool SfxObjectShell::SignDocumentContentUsingCertificate(svl::crypto::SigningCon
     // the document is not new and is not modified
     OUString aODFVersion(comphelper::OStorageHelper::GetODFVersionFromStorage(GetStorage()));
 
+    if (IsModified() && IsSignPDF())
+    {
+        // When signing a PDF, then adding/resizing/moving the signature line would nominally modify
+        // the document, but ignore that for signing.
+        SetModified(false);
+    }
+
     if (IsModified() || !GetMedium() || GetMedium()->GetName().isEmpty()
       || (GetMedium()->GetFilter()->IsOwnFormat() && aODFVersion.compareTo(ODFVER_012_TEXT) < 0 && !bHasSign))
     {