[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: [PATCH] khtml/konqueror download manager integration
From:       Rainer Wirtz <rainer.wirtz () gmx ! de>
Date:       2005-01-30 22:30:05
Message-ID: 200501302330.05364.rainer.wirtz () gmx ! de
[Download RAW message or body]

Am Sonntag, 30. Januar 2005 10:46 schrieb David Faure:

> Thanks, this looks better.
> I wonder about "save background image as"  (slotSaveBackground()),
> File / "save as"   (which calls slotSaveDocument())
> and "Save Frame As" (slotSaveFrame())  :
> why do they use saveURLDirect instead of saveURL? Shouldn't those
> things use the download manager too?
> (Same question about khtmlrun). Basically I'm wondering why any code
> outside of khtml_ext would want to call saveURLDirect instead of
> saveURL?

I did this for two reasons:
save background, save document and save frame don't "feel" like download 
operations to me, because normally the document is already there (in 
cache). OTOH same is true for "save image as".
When a document is in cache and I'm offline when saving (not uncommon 
for me, modem user) then kget is not saving immediately. It adds a 
transfer to it's list and waits until either I go online or I start the 
transfer by hand. Not using kget in this case is more "smooth".

After thinking about it I think The Right Thing(tm) would be to make all 
save operation via kget and to have kget check for availability in 
cache even when offline. And come to think of it check for availabilty 
in cache when online and the new transfer is queued behind already 
running transfers (so it doesn't wait until those transfers are 
finished). 

I changed the patch again and will start looking into changing kget now.
Since saveURL() is now always called, there's no need to have a separat 
saveURLDirect() and I merged the two.
You may want to wait with commiting until kget is fixed.

Note that "konfig-file konquerorrc; group HTML Settings; PathEntry 
DownloadManager" are all hardcoded into khtml, breaking a hypothetical 
kmyperfectbrowser to use khtml.  

While we're at it. I'm very much used to use "RMB->save link as"  on 
html pages. Downloading from ftp has no comparable functionality. I 
either have to use the drop-target or "LMB->save as" in the dialog that 
pops up then (which does not work when offline). I would like to have 
"RMB->copy to->..." use kget as well. I will submit a patch, but only 
after kget has learned to handle folders. Otherwise this would be a 
loss in functionality and users would complain.

Rainer

["khtml_DLM_integration3.diff" (text/x-diff)]

diff -U 3 -H -d -r -N -X /home/ratz/diffexclude -- khtml.orig/khtml_ext.cpp \
                kdelibs/khtml/khtml_ext.cpp
--- khtml.orig/khtml_ext.cpp	2005-01-30 01:34:56.000000000 +0100
+++ kdelibs/khtml/khtml_ext.cpp	2005-01-30 21:56:46.000000000 +0100
@@ -700,29 +700,69 @@
                                    const QString &filter, long cacheId,
                                    const QString & suggestedFilename )
 {
-  QString name = QString::fromLatin1( "index.html" );
-  if ( !suggestedFilename.isEmpty() )
-    name = suggestedFilename;
-  else if ( !url.fileName().isEmpty() )
-    name = url.fileName();
+        // DownloadManager <-> konqueror integration
+        // find if the integration is enabled
+        // the empty key  means no integration
+        // only use download manager for non-local urls!
 
-  KURL destURL;
-  int query;
-  do {
-    query = KMessageBox::Yes;
-    destURL = KFileDialog::getSaveURL( name, filter, parent, caption );
-      if( destURL.isLocalFile() )
-      {
-        QFileInfo info( destURL.path() );
-        if( info.exists() ) {
-          // TODO: use KIO::RenameDlg (shows more information)
-          query = KMessageBox::warningYesNo( parent, i18n( "A file named \"%1\" \
already exists. " "Are you sure you want to overwrite it?" ).arg( info.fileName() ), \
i18n( "Overwrite File?" ), i18n( "Overwrite" ), KStdGuiItem::cancel() ); +    if ( \
!url.isLocalFile() ) +    {
+        // TODO: this shouldn't be hardcoded here
+        KConfig cfg("konquerorrc", false, false);
+        cfg.setGroup("HTML Settings");
+        QString downloadManger = cfg.readPathEntry("DownloadManager");
+        if (!downloadManger.isEmpty())
+        {
+                // then find the download manager location
+            kdDebug(1000) << "Using: "<<downloadManger <<" as Download Manager" \
<<endl; +            QString cmd = KStandardDirs::findExe(downloadManger);
+            if (cmd.isEmpty())
+            {
+                QString errMsg=i18n("The Download Manager (%1) could not be found in \
your $PATH ").arg(downloadManger); +                QString errMsgEx= i18n("Try to \
reinstall it  \n\nThe integration with Konqueror will be disabled!"); +               \
KMessageBox::detailedSorry(0,errMsg,errMsgEx); +                \
cfg.writePathEntry("DownloadManager",QString::null); +                cfg.sync ();
+            }
+            else
+            {
+                KURL _url = url;
+                if ( url.fileName( false ).isEmpty() )
+                {
+                    _url.addPath ("index.html");
+                    QString d_url=_url.prettyURL();
+                }
+                cmd += " " + KProcess::quote(_url.url());
+                kdDebug(1000) << "Calling command  "<<cmd<<endl;
+                KRun::runCommand(cmd);
+                return;
+            }
         }
-       }
-   } while ( query == KMessageBox::No );
+    } //end if ( !url.isLocalFile() )
 
-  if ( destURL.isValid() )
-    saveURL(url, destURL, metadata, cacheId);
+    QString name = QString::fromLatin1( "index.html" );
+    if ( !suggestedFilename.isEmpty() )
+        name = suggestedFilename;
+    else if ( !url.fileName().isEmpty() )
+        name = url.fileName();
+
+    KURL destURL;
+    int query;
+    do {
+        query = KMessageBox::Yes;
+        destURL = KFileDialog::getSaveURL( name, filter, parent, caption );
+        if( destURL.isLocalFile() )
+        {
+            QFileInfo info( destURL.path() );
+            if( info.exists() ) {
+            // TODO: use KIO::RenameDlg (shows more information)
+                query = KMessageBox::warningYesNo( parent, i18n( "A file named \
\"%1\" already exists. " "Are you sure you want to overwrite it?" ).arg( \
info.fileName() ), i18n( "Overwrite File?" ), i18n( "Overwrite" ), \
KStdGuiItem::cancel() ); +            }
+        }
+    } while ( query == KMessageBox::No );
+
+    if ( destURL.isValid() )
+        saveURL(url, destURL, metadata, cacheId);
 }
 
 void KHTMLPopupGUIClient::saveURL( const KURL &url, const KURL &destURL,
@@ -760,50 +800,11 @@
         }
         if(!saved)
         {
-          // DownloadManager <-> konqueror integration
-          // find if the integration is enabled
-          // the empty key  means no integration
-          // only use download manager for non-local urls!
-          bool downloadViaKIO = true;
-          if ( !url.isLocalFile() )
-          {
-            KConfig cfg("konquerorrc", false, false);
-            cfg.setGroup("HTML Settings");
-            QString downloadManger = cfg.readPathEntry("DownloadManager");
-            if (!downloadManger.isEmpty())
-            {
-                // then find the download manager location
-                kdDebug(1000) << "Using: "<<downloadManger <<" as Download Manager" \
                <<endl;
-                QString cmd = KStandardDirs::findExe(downloadManger);
-                if (cmd.isEmpty())
-                {
-                    QString errMsg=i18n("The Download Manager (%1) could not be \
                found in your $PATH ").arg(downloadManger);
-                    QString errMsgEx= i18n("Try to reinstall it  \n\nThe integration \
                with Konqueror will be disabled!");
-                    KMessageBox::detailedSorry(0,errMsg,errMsgEx);
-                    cfg.writePathEntry("DownloadManager",QString::null);
-                    cfg.sync ();
-                }
-                else
-                {
-                    downloadViaKIO = false;
-                    KURL cleanDest = destURL;
-                    cleanDest.setPass( QString::null ); // don't put password into \
                commandline
-                    cmd += " " + KProcess::quote(url.url()) + " " +
-                           KProcess::quote(cleanDest.url());
-                    kdDebug(1000) << "Calling command  "<<cmd<<endl;
-                    KRun::runCommand(cmd);
-                }
-            }
-          }
-
-          if ( downloadViaKIO )
-          {
-              KIO::Job *job = KIO::file_copy( url, destURL, -1, true /*overwrite*/ \
                );
-              job->setMetaData(metadata);
-              job->addMetaData("MaxCacheSize", "0"); // Don't store in http cache.
-              job->addMetaData("cache", "cache"); // Use entry from cache if \
                available.
-              job->setAutoErrorHandlingEnabled( true );
-          }
+            KIO::Job *job = KIO::file_copy( url, destURL, -1, true /*overwrite*/ );
+            job->setMetaData(metadata);
+            job->addMetaData("MaxCacheSize", "0"); // Don't store in http cache.
+            job->addMetaData("cache", "cache"); // Use entry from cache if \
available. +            job->setAutoErrorHandlingEnabled( true );
         } //end if(!saved)
     }
 }
diff -U 3 -H -d -r -N -X /home/ratz/diffexclude -- khtml.orig/khtml_ext.h \
                kdelibs/khtml/khtml_ext.h
--- khtml.orig/khtml_ext.h	2005-01-30 01:34:56.000000000 +0100
+++ kdelibs/khtml/khtml_ext.h	2005-01-30 21:57:25.000000000 +0100
@@ -118,6 +118,7 @@
   KHTMLPopupGUIClient( KHTMLPart *khtml, const QString &doc, const KURL &url );
   virtual ~KHTMLPopupGUIClient();
 
+  // save using download manager if available
   static void saveURL( QWidget *parent, const QString &caption, const KURL &url,
                        const QMap<QString, QString> &metaData = KIO::MetaData(),
                        const QString &filter = QString::null, long cacheId = 0,



>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic