This commit aims to fix all troubles with favicons retrieving.

There is at least just another one about threading. But I'm not 100% sure about.
And anyway, it will be eventually addressed after 0.6 release..

8 files changed, 101 insertions, 42 deletions

diff --git a/src/bookmarks/bookmarkowner.cpp b/src/bookmarks/bookmarkowner.cpp
index 8a05d646..e1b9a567 100644
--- a/src/bookmarks/bookmarkowner.cpp
+++ b/src/bookmarks/bookmarkowner.cpp
@@ -195,8 +195,7 @@ KBookmark BookmarkOwner::bookmarkCurrentPage(const KBookmark &bookmark)
         else
             parent = selected.parentGroup();
 
-        newBk = parent.addBookmark(currentTitle().replace('&', "&&"), KUrl(currentUrl()),
-                                   Application::iconManager()->iconForUrl(currentUrl()).name());
+        newBk = parent.addBookmark(currentTitle().replace('&', "&&"), KUrl(currentUrl()) );
         parent.moveBookmark(newBk, selected);
     }
     else
diff --git a/src/bookmarks/bookmarkprovider.cpp b/src/bookmarks/bookmarkprovider.cpp
index 7cd48825..3e2d2814 100644
--- a/src/bookmarks/bookmarkprovider.cpp
+++ b/src/bookmarks/bookmarkprovider.cpp
@@ -237,7 +237,7 @@ void BookmarkProvider::fillBookmarkBar(BookmarkToolBar *toolBar)
         {
             KBookmarkAction *action = new KBookmarkAction(bookmark, m_owner, this);
             action->setIconText(action->iconText().replace('&', "&&"));
-            action->setIcon(Application::iconManager()->iconForUrl(bookmark.url()));
+            action->setIcon(Application::iconManager()->iconForUrl( KUrl(bookmark.url()) ));
             connect(action, SIGNAL(hovered()), toolBar, SLOT(actionHovered()));
             toolBar->toolBar()->addAction(action);
             toolBar->toolBar()->widgetForAction(action)->installEventFilter(toolBar);
diff --git a/src/bookmarks/bookmarkstoolbar.cpp b/src/bookmarks/bookmarkstoolbar.cpp
index 77bff7e6..4eff2e52 100644
--- a/src/bookmarks/bookmarkstoolbar.cpp
+++ b/src/bookmarks/bookmarkstoolbar.cpp
@@ -91,7 +91,7 @@ QAction * BookmarkMenu::actionForBookmark(const KBookmark &bookmark)
     else
     {
         KBookmarkAction *action = new KBookmarkAction(bookmark, owner(), this);
-        action->setIcon(Application::iconManager()->iconForUrl(bookmark.url()));
+        action->setIcon(Application::iconManager()->iconForUrl( KUrl(bookmark.url()) ));
         connect(action, SIGNAL(hovered()), this, SLOT(actionHovered()));
         return action;
     }
diff --git a/src/iconmanager.cpp b/src/iconmanager.cpp
index f8d1a6a4..c4b9751d 100644
--- a/src/iconmanager.cpp
+++ b/src/iconmanager.cpp
@@ -51,6 +51,7 @@
 IconManager::IconManager(QObject *parent)
     : QObject(parent)
 {
+    _faviconsDir = KStandardDirs::locateLocal("cache" , "favicons/" , true);
 }
 
 
@@ -84,19 +85,20 @@ KIcon IconManager::iconForUrl(const KUrl &url)
     if (encodedUrl == QByteArray("about:downloads"))
         return KIcon("download");
 
+    // TODO: return other mimetype icons
+    if(url.isLocalFile())
+    {
+        return KIcon("folder");
+    }
+    
     QString i = KMimeType::favIconForUrl(url);
-    QString faviconDir = KStandardDirs::locateLocal("cache" , "" , true);
     if(!i.isEmpty())
     {
+        QString faviconDir = KStandardDirs::locateLocal("cache" , "" , true);
         return KIcon(faviconDir + i);
     }
+    
     kDebug() << "Icon NOT Found. returning text-html one";
-
-    // TODO: return other mimetype icons
-    if(url.isLocalFile())
-    {
-        return KIcon("folder");
-    }
     return KIcon("text-html");
 }
 
@@ -121,13 +123,6 @@ void IconManager::provideIcon(QWebPage *page, const KUrl &url, bool notify)
         return;
     }
     
-    QUrl u(url.url());
-    QString rootUrlString = u.toString(  QUrl::RemovePassword
-                                       | QUrl::RemoveUserInfo
-                                       | QUrl::RemovePath
-                                       | QUrl::RemoveQuery
-                                       | QUrl::StripTrailingSlash);
-
     // check if icon exists
     if(!KMimeType::favIconForUrl(url).isEmpty())
     {
@@ -137,8 +132,11 @@ void IconManager::provideIcon(QWebPage *page, const KUrl &url, bool notify)
         return;
     }
 
-    // find ico url
-    KUrl iconUrl(rootUrlString + QL1S("/favicon.ico"));
+    // the simplest way..
+    const QString rootUrlString = url.scheme() + QL1S("://") + url.host();  
+
+    // find favicon url
+    KUrl faviconUrl( rootUrlString + QL1S("/favicon.ico") );
 
     QWebElement root = page->mainFrame()->documentElement();
     QWebElement e = root.findFirst(QL1S("link[rel~=\"icon\"]"));
@@ -146,28 +144,28 @@ void IconManager::provideIcon(QWebPage *page, const KUrl &url, bool notify)
     if(relUrlString.isEmpty())
     {
         e = root.findFirst(QL1S("link[rel~=\"shortcut icon\"]"));
-        relUrlString = e.attribute(QL1S("href"));
+        relUrlString = e.attribute(QL1S("href"));        
     }
 
     if(!relUrlString.isEmpty())
     {
-        iconUrl = relUrlString.startsWith("http")
-                ? KUrl(relUrlString)
-                : KUrl(rootUrlString + relUrlString) ;
+        faviconUrl = relUrlString.startsWith("http")
+                    ? KUrl(relUrlString)
+                    : KUrl(rootUrlString + QL1C('/') + relUrlString) ;
     }
 
-    kDebug() << "ICON URL: " << iconUrl;
+    kDebug() << "ICON URL: " << faviconUrl;
 
-    QString faviconDir = KStandardDirs::locateLocal("cache" , "favicons/" , true);
-
-    int r = rootUrlString.indexOf(':');
-    KUrl destUrl(faviconDir + rootUrlString.mid(r+3) + ".png");
+    // dest url
+    KUrl destUrl(_faviconsDir + url.host() + QL1S(".png") );
     kDebug() << "DEST URL: " << destUrl;
 
     // download icon
-    KIO::FileCopyJob *job = KIO::file_copy(iconUrl, destUrl, -1, KIO::HideProgressInfo);
+    KIO::FileCopyJob *job = KIO::file_copy(faviconUrl, destUrl, -1, KIO::HideProgressInfo);
     if(notify)
-        connect(job, SIGNAL(result(KJob*)), this, SIGNAL(iconChanged()));
+        connect(job, SIGNAL(result(KJob*)), this, SLOT(notifyLastStuffs(KJob *)));
+    else
+        connect(job, SIGNAL(result(KJob*)), this, SLOT(doLastStuffs(KJob *)));
 }
 
 
@@ -179,11 +177,66 @@ void IconManager::downloadIconFromUrl(const KUrl &url)
 
 void IconManager::clearIconCache()
 {
-    QString faviconDir = KStandardDirs::locateLocal("cache" , "favicons/" , true);
-    QDir d(faviconDir);
+    QDir d(_faviconsDir);
     QStringList favicons = d.entryList();
     Q_FOREACH(const QString &fav, favicons)
     {
         d.remove(fav);
     }
 }
+
+
+void IconManager::doLastStuffs(KJob *j)
+{
+    if(j->error())
+    {
+        kDebug() << "FAVICON JOB ERROR";
+        return;
+    }
+    
+    KIO::FileCopyJob *job = static_cast<KIO::FileCopyJob *>(j);
+    KUrl dest = job->destUrl();
+
+    QString s = dest.url().remove( QL1S("file://") );
+    QFile fav(s);
+    if(!fav.exists())
+    {
+        kDebug() << "FAVICON DOES NOT EXISTS";
+        fav.remove();
+        return;
+    }
+    
+    if(fav.size() == 0)
+    {
+        kDebug() << "SIZE ZERO FAVICON";
+        fav.remove();
+        return;
+    }
+    
+    QPixmap px;
+    if(!px.load(s))
+    {
+        kDebug() << "PIXMAP NOT LOADED";
+        return;
+    }
+    
+    if(px.isNull())
+    {
+        kDebug() << "PIXMAP IS NULL";
+        fav.remove();
+        return;
+    }
+    
+    px = px.scaled(16,16);
+    if(!px.save(s))
+    {
+        kDebug() << "PIXMAP NOT SAVED";
+        return;
+    }
+}
+
+void IconManager::notifyLastStuffs(KJob *j)
+{
+    doLastStuffs(j);
+    emit iconChanged();
+}
diff --git a/src/iconmanager.h b/src/iconmanager.h
index 9e451431..724d631a 100644
--- a/src/iconmanager.h
+++ b/src/iconmanager.h
@@ -54,9 +54,18 @@ public:
     void downloadIconFromUrl(const KUrl &url);
 
     void clearIconCache();
-    
+
+private Q_SLOTS:
+    void doLastStuffs(KJob *);
+    void notifyLastStuffs(KJob *);
+        
 Q_SIGNALS:
     void iconChanged();
+    
+private:
+    bool existsIconForUrl(const KUrl &url);
+
+    QString _faviconsDir;
 };
 
 
diff --git a/src/mainview.cpp b/src/mainview.cpp
index c9f46e5a..f020fb53 100644
--- a/src/mainview.cpp
+++ b/src/mainview.cpp
@@ -321,6 +321,7 @@ WebTab *MainView::newWebTab(bool focused)
     connect(tab->view(), SIGNAL(loadFinished(bool)), this, SLOT(webViewLoadFinished(bool)));
     connect(tab->view(), SIGNAL(titleChanged(const QString &)), this, SLOT(webViewTitleChanged(const QString &)));
     connect(tab->view(), SIGNAL(urlChanged(const QUrl &)), this, SLOT(webViewUrlChanged(const QUrl &)));
+    connect(tab->view(), SIGNAL(iconChanged()), this, SLOT(webViewIconChanged()));
 
     // connecting webPage signals with mainview
     connect(tab->view()->page(), SIGNAL(windowCloseRequested()), this, SLOT(windowCloseRequested()));
@@ -631,6 +632,7 @@ void MainView::previousTab()
     setCurrentIndex(next);
 }
 
+
 void MainView::openClosedTabs()
 {
     foreach (const HistoryItem &item, recentlyClosedTabs())
@@ -640,6 +642,7 @@ void MainView::openClosedTabs()
     m_recentlyClosedTabs.clear();
 }
 
+
 void MainView::openClosedTab()
 {
     KAction *action = qobject_cast<KAction *>(sender());
@@ -654,6 +657,7 @@ void MainView::openClosedTab()
     }
 }
 
+
 void MainView::switchToTab()
 {
     // uses the sender to determine the tab index
@@ -665,6 +669,7 @@ void MainView::switchToTab()
     setCurrentIndex( index );
 }
 
+
 QLabel *MainView::animatedLoading(int index, bool addMovie)
 {
     if (index == -1)
diff --git a/src/webpage.cpp b/src/webpage.cpp
index 4c720767..07f961c5 100644
--- a/src/webpage.cpp
+++ b/src/webpage.cpp
@@ -208,7 +208,6 @@ WebPage::WebPage(QWidget *parent)
     // ----- last stuffs
     connect(manager, SIGNAL(finished(QNetworkReply*)), this, SLOT(manageNetworkErrors(QNetworkReply*)));
     connect(this, SIGNAL(loadFinished(bool)), this, SLOT(loadFinished(bool)));
-    connect(this, SIGNAL(loadStarted()), this, SLOT(loadStarted()));
 
     // protocol handler signals
     connect(&_protHandler, SIGNAL(downloadUrl(const KUrl &)), this, SLOT(downloadUrl(const KUrl &)));
@@ -472,15 +471,10 @@ void WebPage::handleUnsupportedContent(QNetworkReply *reply)
 }
 
 
-void WebPage::loadStarted()
-{
-    Application::iconManager()->provideIcon(this, _loadingUrl);
-}
-
-
 void WebPage::loadFinished(bool ok)
 {
     Q_UNUSED(ok);
+    Application::iconManager()->provideIcon(this, _loadingUrl);
 
 
     Application::adblockManager()->applyHidingRules(this);
diff --git a/src/webpage.h b/src/webpage.h
index ed5e2dde..5b79275a 100644
--- a/src/webpage.h
+++ b/src/webpage.h
@@ -75,7 +75,6 @@ private slots:
     void handleUnsupportedContent(QNetworkReply *reply);
     void manageNetworkErrors(QNetworkReply *reply);
     void loadFinished(bool);
-    void loadStarted();
     void showSSLInfo(QPoint);
     void updateImage(bool ok);