Merge pull request #1231 from kiwix/Bugfix-save-page-incorrect-cancel

BugFix: HTML Web Page incorrect behavior when canceling save page actions
kiwix · Oct 30, 2024 · d8b05ae · d8b05ae
2 parents 0585c34 + 3ebaaaa
commit d8b05ae
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 30 deletions.
diff --git a/src/kprofile.cpp b/src/kprofile.cpp
@@ -5,6 +5,26 @@
 #include <QMessageBox>
 #include <QWebEngineSettings>
 
+QString askForSaveFilePath(const QString& suggestedName)
+{
+    const auto app = KiwixApp::instance();
+    const QString suggestedPath = app->getPrevSaveDir() + "/" + suggestedName;
+    const QString extension = suggestedName.section(".", -1);
+    const QString filter = extension.isEmpty() ? "" : "(*" + extension + ")";
+    QString fileName = QFileDialog::getSaveFileName(
+            app->getMainWindow(), gt("save-file-as-window-title"),
+            suggestedPath, filter);
+
+    if (fileName.isEmpty())
+        return QString();
+
+    if (!fileName.endsWith(extension)) {
+        fileName.append(extension);
+    }
+    app->savePrevSaveDir(QFileInfo(fileName).absolutePath());
+    return fileName;
+}
+
 KProfile::KProfile(QObject *parent) :
     QWebEngineProfile(parent)
 {
@@ -24,32 +44,16 @@ void KProfile::startDownload(QWebEngineDownloadItem* download)
 void KProfile::startDownload(QWebEngineDownloadRequest* download)
 #endif
 {
-    auto app = KiwixApp::instance();
 #if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
     QString defaultFileName = QUrl(download->path()).fileName();
 #else
     QString defaultFileName = download->downloadFileName();
 #endif
-    QString suggestedPath = app->getPrevSaveDir() + "/" + defaultFileName;
-    QString extension = defaultFileName.section('.', -1);
-    QString filter = extension != '.' ? "(*" + extension + ")" : "";
-
-    QString fileName = QFileDialog::getSaveFileName(
-            app->getMainWindow(), gt("save-file-as-window-title"),
-            suggestedPath, filter);
-
+    const QString fileName = askForSaveFilePath(defaultFileName);
     if (fileName.isEmpty()) {
         return;
     }
-    if (!fileName.endsWith(extension)) {
-        fileName.append(extension);
-    }
-    app->savePrevSaveDir(QFileInfo(fileName).absolutePath());
 
-    if (download->isSavePageDownload()) {
-        download->page()->printToPdf(fileName);
-        return;
-    }
 #if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
     download->setPath(fileName);
 #else

diff --git a/src/webview.cpp b/src/webview.cpp
@@ -18,6 +18,7 @@ class QMenu;
 #include <kiwix/tools.h>
 
 zim::Entry getArchiveEntryFromUrl(const zim::Archive& archive, const QUrl& url);
+QString askForSaveFilePath(const QString& suggestedName);
 
 void WebViewBackMenu::showEvent(QShowEvent *)
 {
@@ -138,26 +139,51 @@ QMenu* WebView::getHistoryForwardMenu() const
     return ret;
 }
 
+namespace
+{
+
+/**
+ * @brief Get the Zim Item object corresponding to the given url.
+ * 
+ * @param url QUrl
+ * @return zim::Item
+ * 
+ * @exception throws exception if zimId is invalid, archive doesn't exist,
+ * entry is invalid or not found, or entry is redirect.
+ */
+zim::Item getZimItem(const QUrl& url)
+{
+    const auto app = KiwixApp::instance();
+    const auto library = app->getLibrary();
+    const auto archive = library->getArchive(getZimIdFromUrl(url));
+    const auto entry = getArchiveEntryFromUrl(*archive, url);
+    return entry.getItem(true);
+}
+
+bool isHTMLContent(const zim::Item& item)
+{
+    auto mimeType = QByteArray::fromStdString(item.getMimetype());
+    mimeType = mimeType.split(';')[0];
+    return mimeType == "text/html";
+}
+
+}
+
 void WebView::saveViewContent()
 {
     try {
-        auto app = KiwixApp::instance();
-        auto library = app->getLibrary();
-        auto archive = library->getArchive(m_currentZimId);
-        auto entry = getArchiveEntryFromUrl(*archive, this->url());
-        if (entry.isRedirect()) 
-            return;
-
-        auto item = entry.getItem(true);
-        auto mimeType = QByteArray::fromStdString(item.getMimetype());
-        mimeType = mimeType.split(';')[0];
+        const auto item = getZimItem(url());
 
         /* We have to sanitize here, as parsing will start once we pass the file
            name to either save or download method.
         */
-        QString suggestedFileName = QString::fromStdString(kiwix::getSlugifiedFileName(item.getTitle()));
-        if (mimeType == "text/html")
-            page()->save(suggestedFileName + ".pdf");
+        const QString suggestedFileName = QString::fromStdString(kiwix::getSlugifiedFileName(item.getTitle()));
+        if (isHTMLContent(item))
+        {
+            const QString fileName = askForSaveFilePath(suggestedFileName + ".pdf");
+            if (!fileName.isEmpty())
+                page()->printToPdf(fileName);
+        }
         else
             page()->download(this->url(), suggestedFileName);
     }