qt5/6: Make Annotation::setModification/CreationDate work on existing annots

With an autotest

and bonus memory leak fixes for existing tests

4 files changed, 190 insertions, 34 deletions

diff --git a/qt5/src/poppler-annotation.cc b/qt5/src/poppler-annotation.cc
index 79280023..c9af2210 100644
--- a/qt5/src/poppler-annotation.cc
+++ b/qt5/src/poppler-annotation.cc
@@ -52,6 +52,7 @@
 #include <Error.h>
 #include <FileSpec.h>
 #include <Link.h>
+#include <DateInfo.h>
 
 /* Almost all getters directly query the underlying poppler annotation, with
  * the exceptions of link, file attachment, sound, movie and screen annotations,
@@ -1342,15 +1343,16 @@ void Annotation::setModificationDate(const QDateTime &date)
         return;
     }
 
-#if 0 // TODO: Conversion routine is broken
-    if (d->pdfAnnot)
-    {
-        time_t t = date.toTime_t();
-        GooString *s = timeToDateString(&t);
-        d->pdfAnnot->setModified(s);
-        delete s;
+    if (d->pdfAnnot) {
+        if (date.isValid()) {
+            const time_t t = date.toTime_t();
+            GooString *s = timeToDateString(&t);
+            d->pdfAnnot->setModified(s);
+            delete s;
+        } else {
+            d->pdfAnnot->setModified(nullptr);
+        }
     }
-#endif
 }
 
 QDateTime Annotation::creationDate() const
@@ -1377,16 +1379,17 @@ void Annotation::setCreationDate(const QDateTime &date)
         return;
     }
 
-#if 0 // TODO: Conversion routine is broken
-    AnnotMarkup *markupann = dynamic_cast<AnnotMarkup*>(d->pdfAnnot);
-    if (markupann)
-    {
-        time_t t = date.toTime_t();
-        GooString *s = timeToDateString(&t);
-        markupann->setDate(s);
-        delete s;
+    AnnotMarkup *markupann = dynamic_cast<AnnotMarkup *>(d->pdfAnnot);
+    if (markupann) {
+        if (date.isValid()) {
+            const time_t t = date.toTime_t();
+            GooString *s = timeToDateString(&t);
+            markupann->setDate(s);
+            delete s;
+        } else {
+            markupann->setDate(nullptr);
+        }
     }
-#endif
 }
 
 static int fromPdfFlags(int flags)
diff --git a/qt5/tests/check_annotations.cpp b/qt5/tests/check_annotations.cpp
index c76e745c..266b8d57 100644
--- a/qt5/tests/check_annotations.cpp
+++ b/qt5/tests/check_annotations.cpp
@@ -16,11 +16,15 @@ class TestAnnotations : public QObject
     Q_OBJECT
 public:
     TestAnnotations(QObject *parent = nullptr) : QObject(parent) { }
+
+    void saveAndCheck(const std::unique_ptr<Poppler::Document> &doc, const std::function<void(Poppler::Annotation *a)> &checkFunction);
+
 private slots:
     void checkQColorPrecision();
     void checkFontSizeAndColor();
     void checkHighlightFromAndToQuads();
     void checkUTF16LEAnnot();
+    void checkModificationCreationDate();
     void checkNonMarkupAnnotations();
     void checkDefaultAppearance();
 };
@@ -137,6 +141,76 @@ void TestAnnotations::checkUTF16LEAnnot()
 
     auto annot = annots[1];
     QCOMPARE(annot->contents(), QString::fromUtf8("Únîcödé豰")); // clazy:exclude=qstring-allocations
+
+    qDeleteAll(annots);
+}
+
+void TestAnnotations::saveAndCheck(const std::unique_ptr<Poppler::Document> &doc, const std::function<void(Poppler::Annotation *a)> &checkFunction)
+{
+    // also check that saving yields the same output
+    QTemporaryFile tempFile;
+    QVERIFY(tempFile.open());
+    tempFile.close();
+
+    std::unique_ptr<Poppler::PDFConverter> conv(doc->pdfConverter());
+    conv->setOutputFileName(tempFile.fileName());
+    conv->setPDFOptions(Poppler::PDFConverter::WithChanges);
+    conv->convert();
+
+    std::unique_ptr<Poppler::Document> savedDoc { Poppler::Document::load(tempFile.fileName()) };
+    std::unique_ptr<Poppler::Page> page { doc->page(0) };
+    auto annots = page->annotations();
+    checkFunction(annots.at(1));
+    qDeleteAll(annots);
+}
+
+void TestAnnotations::checkModificationCreationDate()
+{
+    std::unique_ptr<Poppler::Document> doc { Poppler::Document::load(TESTDATADIR "/unittestcases/utf16le-annot.pdf") };
+    QVERIFY(doc.get());
+
+    std::unique_ptr<Poppler::Page> page { doc->page(0) };
+
+    auto annots = page->annotations();
+    auto annot = annots.at(1);
+    QCOMPARE(annot->creationDate(), QDateTime());
+    QCOMPARE(annot->modificationDate(), QDateTime());
+
+    const QDateTime dt1(QDate(2020, 8, 7), QTime(18, 34, 56));
+    annot->setCreationDate(dt1);
+    auto checkFunction1 = [dt1](Poppler::Annotation *a) {
+        QCOMPARE(a->creationDate(), dt1);
+        // setting the creation date updates the modification date
+        QVERIFY(std::abs(a->modificationDate().secsTo(QDateTime::currentDateTime())) < 2);
+    };
+    checkFunction1(annot);
+    saveAndCheck(doc, checkFunction1);
+
+    const QDateTime dt2(QDate(2020, 8, 30), QTime(8, 14, 52));
+    annot->setModificationDate(dt2);
+    auto checkFunction2 = [dt2](Poppler::Annotation *a) { QCOMPARE(a->modificationDate(), dt2); };
+    checkFunction2(annot);
+    saveAndCheck(doc, checkFunction2);
+
+    // setting the creation date to empty means "use the modification date" and also updates the modification date
+    // so both creation date and modification date are the same and are now
+    annot->setCreationDate(QDateTime());
+    auto checkFunction3 = [](Poppler::Annotation *a) {
+        QVERIFY(std::abs(a->creationDate().secsTo(QDateTime::currentDateTime())) < 2);
+        QCOMPARE(a->creationDate(), a->modificationDate());
+    };
+    checkFunction3(annot);
+    saveAndCheck(doc, checkFunction3);
+
+    annot->setModificationDate(QDateTime());
+    auto checkFunction4 = [](Poppler::Annotation *a) {
+        QCOMPARE(a->creationDate(), QDateTime());
+        QCOMPARE(a->modificationDate(), QDateTime());
+    };
+    checkFunction4(annot);
+    saveAndCheck(doc, checkFunction4);
+
+    qDeleteAll(annots);
 }
 
 void TestAnnotations::checkNonMarkupAnnotations()
@@ -149,6 +223,7 @@ void TestAnnotations::checkNonMarkupAnnotations()
 
     auto annots = page->annotations();
     QCOMPARE(annots.size(), 17);
+    qDeleteAll(annots);
 }
 
 void TestAnnotations::checkDefaultAppearance()
diff --git a/qt6/src/poppler-annotation.cc b/qt6/src/poppler-annotation.cc
index fd77a651..f12a9181 100644
--- a/qt6/src/poppler-annotation.cc
+++ b/qt6/src/poppler-annotation.cc
@@ -51,6 +51,7 @@
 #include <Error.h>
 #include <FileSpec.h>
 #include <Link.h>
+#include <DateInfo.h>
 
 /* Almost all getters directly query the underlying poppler annotation, with
  * the exceptions of link, file attachment, sound, movie and screen annotations,
@@ -1030,15 +1031,16 @@ void Annotation::setModificationDate(const QDateTime &date)
         return;
     }
 
-#if 0 // TODO: Conversion routine is broken
-    if (d->pdfAnnot)
-    {
-        time_t t = date.toTime_t();
-        GooString *s = timeToDateString(&t);
-        d->pdfAnnot->setModified(s);
-        delete s;
+    if (d->pdfAnnot) {
+        if (date.isValid()) {
+            const time_t t = date.toSecsSinceEpoch();
+            GooString *s = timeToDateString(&t);
+            d->pdfAnnot->setModified(s);
+            delete s;
+        } else {
+            d->pdfAnnot->setModified(nullptr);
+        }
     }
-#endif
 }
 
 QDateTime Annotation::creationDate() const
@@ -1065,16 +1067,17 @@ void Annotation::setCreationDate(const QDateTime &date)
         return;
     }
 
-#if 0 // TODO: Conversion routine is broken
-    AnnotMarkup *markupann = dynamic_cast<AnnotMarkup*>(d->pdfAnnot);
-    if (markupann)
-    {
-        time_t t = date.toTime_t();
-        GooString *s = timeToDateString(&t);
-        markupann->setDate(s);
-        delete s;
+    AnnotMarkup *markupann = dynamic_cast<AnnotMarkup *>(d->pdfAnnot);
+    if (markupann) {
+        if (date.isValid()) {
+            const time_t t = date.toSecsSinceEpoch();
+            GooString *s = timeToDateString(&t);
+            markupann->setDate(s);
+            delete s;
+        } else {
+            markupann->setDate(nullptr);
+        }
     }
-#endif
 }
 
 static Annotation::Flags fromPdfFlags(int flags)
diff --git a/qt6/tests/check_annotations.cpp b/qt6/tests/check_annotations.cpp
index 68e35a3d..c8f38044 100644
--- a/qt6/tests/check_annotations.cpp
+++ b/qt6/tests/check_annotations.cpp
@@ -16,11 +16,15 @@ class TestAnnotations : public QObject
     Q_OBJECT
 public:
     TestAnnotations(QObject *parent = nullptr) : QObject(parent) { }
+
+    void saveAndCheck(const std::unique_ptr<Poppler::Document> &doc, const std::function<void(Poppler::Annotation *a)> &checkFunction);
+
 private slots:
     void checkQColorPrecision();
     void checkFontSizeAndColor();
     void checkHighlightFromAndToQuads();
     void checkUTF16LEAnnot();
+    void checkModificationCreationDate();
     void checkNonMarkupAnnotations();
     void checkDefaultAppearance();
 };
@@ -137,6 +141,76 @@ void TestAnnotations::checkUTF16LEAnnot()
 
     auto annot = annots[1];
     QCOMPARE(annot->contents(), QString::fromUtf8("Únîcödé豰")); // clazy:exclude=qstring-allocations
+
+    qDeleteAll(annots);
+}
+
+void TestAnnotations::saveAndCheck(const std::unique_ptr<Poppler::Document> &doc, const std::function<void(Poppler::Annotation *a)> &checkFunction)
+{
+    // also check that saving yields the same output
+    QTemporaryFile tempFile;
+    QVERIFY(tempFile.open());
+    tempFile.close();
+
+    std::unique_ptr<Poppler::PDFConverter> conv(doc->pdfConverter());
+    conv->setOutputFileName(tempFile.fileName());
+    conv->setPDFOptions(Poppler::PDFConverter::WithChanges);
+    conv->convert();
+
+    std::unique_ptr<Poppler::Document> savedDoc { Poppler::Document::load(tempFile.fileName()) };
+    std::unique_ptr<Poppler::Page> page { doc->page(0) };
+    auto annots = page->annotations();
+    checkFunction(annots.at(1));
+    qDeleteAll(annots);
+}
+
+void TestAnnotations::checkModificationCreationDate()
+{
+    std::unique_ptr<Poppler::Document> doc { Poppler::Document::load(TESTDATADIR "/unittestcases/utf16le-annot.pdf") };
+    QVERIFY(doc.get());
+
+    std::unique_ptr<Poppler::Page> page { doc->page(0) };
+
+    auto annots = page->annotations();
+    auto annot = annots.at(1);
+    QCOMPARE(annot->creationDate(), QDateTime());
+    QCOMPARE(annot->modificationDate(), QDateTime());
+
+    const QDateTime dt1(QDate(2020, 8, 7), QTime(18, 34, 56));
+    annot->setCreationDate(dt1);
+    auto checkFunction1 = [dt1](Poppler::Annotation *a) {
+        QCOMPARE(a->creationDate(), dt1);
+        // setting the creation date updates the modification date
+        QVERIFY(std::abs(a->modificationDate().secsTo(QDateTime::currentDateTime())) < 2);
+    };
+    checkFunction1(annot);
+    saveAndCheck(doc, checkFunction1);
+
+    const QDateTime dt2(QDate(2020, 8, 30), QTime(8, 14, 52));
+    annot->setModificationDate(dt2);
+    auto checkFunction2 = [dt2](Poppler::Annotation *a) { QCOMPARE(a->modificationDate(), dt2); };
+    checkFunction2(annot);
+    saveAndCheck(doc, checkFunction2);
+
+    // setting the creation date to empty means "use the modification date" and also updates the modification date
+    // so both creation date and modification date are the same and are now
+    annot->setCreationDate(QDateTime());
+    auto checkFunction3 = [](Poppler::Annotation *a) {
+        QVERIFY(std::abs(a->creationDate().secsTo(QDateTime::currentDateTime())) < 2);
+        QCOMPARE(a->creationDate(), a->modificationDate());
+    };
+    checkFunction3(annot);
+    saveAndCheck(doc, checkFunction3);
+
+    annot->setModificationDate(QDateTime());
+    auto checkFunction4 = [](Poppler::Annotation *a) {
+        QCOMPARE(a->creationDate(), QDateTime());
+        QCOMPARE(a->modificationDate(), QDateTime());
+    };
+    checkFunction4(annot);
+    saveAndCheck(doc, checkFunction4);
+
+    qDeleteAll(annots);
 }
 
 void TestAnnotations::checkNonMarkupAnnotations()
@@ -149,6 +223,7 @@ void TestAnnotations::checkNonMarkupAnnotations()
 
     auto annots = page->annotations();
     QCOMPARE(annots.size(), 17);
+    qDeleteAll(annots);
 }
 
 void TestAnnotations::checkDefaultAppearance()