Improves caching api related to urls, implements fragment handling, fixes #237

5 files changed, 60 insertions, 37 deletions

diff --git a/src/browsertab.cpp b/src/browsertab.cpp
index f790e8e..1b90720 100644
--- a/src/browsertab.cpp
+++ b/src/browsertab.cpp
@@ -586,6 +586,15 @@ void BrowserTab::on_requestComplete(const QByteArray &ref_data, const MimeType &
 
     renderPage(data, mime);
 
+    if(this->navigate_to_fragment)
+    {
+        // Implement navigation semantics:
+        QString fragment = this->current_location.fragment();
+        if(not fragment.isEmpty()) {
+            this->scrollToAnchor(fragment);
+        }
+    }
+
     this->updatePageTitle();
 
     this->updateUrlBarStyle();
@@ -1585,6 +1594,18 @@ bool BrowserTab::startRequest(const QUrl &url, ProtocolHandler::RequestOptions o
 
     QString urlstr = url.toString(QUrl::FullyEncoded);
 
+    {
+        QUrl old_url_cleaned = this->current_location;
+        QUrl new_url_cleaned = url;
+
+        old_url_cleaned.setFragment("");
+        new_url_cleaned.setFragment("");
+
+        // Only jump to (potential) fragment when we either change the base url
+        // or reload the current page with a new fragment.
+        this->navigate_to_fragment = ((old_url_cleaned != new_url_cleaned) or (this->current_location.fragment() != url.fragment()));
+    }
+
     this->is_internal_location = (url.scheme() == "about" || url.scheme() == "file");
     this->current_location = url;
     this->setUrlBarText(urlstr);
@@ -1596,8 +1617,7 @@ bool BrowserTab::startRequest(const QUrl &url, ProtocolHandler::RequestOptions o
         return this->current_handler->startRequest(url.adjusted(QUrl::RemoveFragment), options);
     };
 
-    if ((flags & RequestFlags::DontReadFromCache) ||
-        this->current_identity.isValid())
+    if ((flags & RequestFlags::DontReadFromCache) || this->current_identity.isValid())
     {
         return req();
     }
@@ -1610,10 +1630,10 @@ bool BrowserTab::startRequest(const QUrl &url, ProtocolHandler::RequestOptions o
         this->was_read_from_cache = true;
         this->on_requestComplete(pg->body, pg->mime);
 
-        // Move scrollbar to cached position
-        if ((flags & RequestFlags::NavigatedBackOrForward) &&
-            pg->scroll_pos != -1)
+        // Move scrollbar to cached position, but only if url or the fragment has changed
+        if ((flags & RequestFlags::NavigatedBackOrForward) && pg->scroll_pos != -1) {
             this->ui->text_browser->verticalScrollBar()->setValue(pg->scroll_pos);
+        }
 
         return true;
     }
diff --git a/src/browsertab.hpp b/src/browsertab.hpp
index 868e519..e862749 100644
--- a/src/browsertab.hpp
+++ b/src/browsertab.hpp
@@ -257,6 +257,9 @@ public:
     RequestState request_state;
 
     DocumentStyle current_style;
+
+    //! If set, `on_requestComplete` will navigate to the fragment of the url.
+    bool navigate_to_fragment = false;
 };
 
 #endif // BROWSERTAB_HPP
diff --git a/src/cachehandler.cpp b/src/cachehandler.cpp
index 35447f9..fc23681 100644
--- a/src/cachehandler.cpp
+++ b/src/cachehandler.cpp
@@ -4,6 +4,11 @@
 
 #include <QDebug>
 
+QString CacheHandler::cleanUrl(QUrl const & url)
+{
+    return url.toString(QUrl::FullyEncoded | QUrl::RemoveFragment);
+}
+
 void CacheHandler::push(const QUrl &url, const QByteArray &body, const MimeType &mime)
 {
     // Skip if this item is above the cached item size threshold
@@ -20,48 +25,40 @@ void CacheHandler::push(const QUrl &url, const QByteArray &body, const MimeType
         this->popOldest();
     }
 
-    QString urlstr = url.toString(QUrl::FullyEncoded | QUrl::RemoveFragment);
+    QString urlstr = cleanUrl(url);
 
-    if (this->page_cache.find(urlstr) != this->page_cache.end())
+    auto it = this->page_cache.find(urlstr);
+    if (it != this->page_cache.end())
     {
-        qDebug() << "cache: updating page";
-        auto pg = this->page_cache[urlstr];
+        qDebug() << "cache: updating page" << urlstr;
+        auto pg = it->second;
         pg->body = body;
         pg->mime = mime;
         pg->time_cached = QDateTime::currentDateTime();
-        return;
     }
-
-    this->page_cache[urlstr] = std::make_shared<CachedPage>(
-        url, body, mime, QDateTime::currentDateTime());
-
-    qDebug() << "cache: pushing url " << url;
-
-    return;
-}
-
-std::shared_ptr<CachedPage> CacheHandler::find(const QString &url)
-{
-    if (this->page_cache.find(url) != this->page_cache.end())
+    else
     {
-        return this->page_cache[url];
+        this->page_cache.emplace(urlstr, std::make_shared<CachedPage>(url, body, mime, QDateTime::currentDateTime()));
+        qDebug() << "cache: pushing url " << urlstr;
     }
-    return nullptr;
 }
 
+
 std::shared_ptr<CachedPage> CacheHandler::find(const QUrl &url)
 {
-    return this->find(url.toString(QUrl::FullyEncoded | QUrl::RemoveFragment));
-}
+    QString urltext = cleanUrl(url);
 
-bool CacheHandler::contains(const QString &url)
-{
-    return this->page_cache.find(url) != this->page_cache.end();
+    auto it = this->page_cache.find(urltext);
+    if(it != this->page_cache.end())
+    {
+        return it->second;
+    }
+    return nullptr;
 }
 
 bool CacheHandler::contains(const QUrl &url)
 {
-    return this->contains(url.toString(QUrl::FullyEncoded | QUrl::RemoveFragment));
+    return this->page_cache.find(cleanUrl(url)) != this->page_cache.end();
 }
 
 int CacheHandler::size()
diff --git a/src/cachehandler.hpp b/src/cachehandler.hpp
index 0d20962..e744f92 100644
--- a/src/cachehandler.hpp
+++ b/src/cachehandler.hpp
@@ -45,7 +45,9 @@ struct CachedPage
     CachedPage(const QUrl &url, const QByteArray &body,
         const MimeType &mime, const QDateTime &cached)
         : url(url), body(body), mime(mime), scroll_pos(-1), time_cached(cached)
-    {}
+    {
+        this->url.setFragment("");
+    }
 };
 
 // Maybe unordered_map isn't the best type for this?
@@ -67,12 +69,10 @@ public:
     CacheMap const& getPages() const;
 
 private:
-    std::shared_ptr<CachedPage> find(QString const &url);
-
-    bool contains(QString const & url);
-
     void popOldest();
 
+    static QString cleanUrl(QUrl const & str);
+
 private:
     // In-memory cache storage.
     CacheMap page_cache;
diff --git a/src/renderers/htmlrenderer.cpp b/src/renderers/htmlrenderer.cpp
index 23f720e..c6e15ad 100644
--- a/src/renderers/htmlrenderer.cpp
+++ b/src/renderers/htmlrenderer.cpp
@@ -109,6 +109,10 @@ static void renderRecursive(RenderState & state, GumboNode const & node, int nes
         bool process_header = false;
         QString header_text;
 
+        // Fetch the original `id` attribute if any
+        char const * const header_id = getAttribute(element, "id");
+        QString const anchor = (header_id != nullptr) ? QString(header_id) : QString("header-%1").arg(state.header_count);
+
         switch(element.tag) {
 
         // Stripped tags
@@ -173,7 +177,7 @@ static void renderRecursive(RenderState & state, GumboNode const & node, int nes
                 process_header = true;
                 state.header_text = &header_text;
             }
-            stream += "<" + QString::fromUtf8(gumbo_normalized_tagname(element.tag)) + QString(" id=\"header-%1\">").arg(state.header_count);
+            stream += "<" + QString::fromUtf8(gumbo_normalized_tagname(element.tag)) + " id=\"" + anchor + "\">";
             break;
 
         default:
@@ -192,7 +196,6 @@ static void renderRecursive(RenderState & state, GumboNode const & node, int nes
             QRegularExpression regex { "\\s+", QRegularExpression::DotMatchesEverythingOption };
 
             QString const header = header_text.replace(regex, " ");
-            QString const anchor = QString("header-%1").arg(state.header_count);
 
             switch(element.tag) {
             case GUMBO_TAG_H1: