[Patch] Improving kwallet support in KHTMLPart

[Eduardo Robles Elvira]

Hello everyone!

Here comes a patch that improves current Kwallet support in KHTMLPart
by replacing the current dialog asking user if he wants to store the
entered password with a topViewBar. It also adds some more
functionality to kwallet icon in the status bar, adding some new
actions to the popup menu shown when right-clicking the kwallet icon,
giving the user the opportunity to change his mind after selecting the
option "Never store passwords for this site" with the action "Allow
storing passwords for this site", and allwoing the user to remove
currently stored passwords for current site within konqueror with
action of type "Remove password for form http://blahblah".

Later I want to add further improvements to the kwallet-khtmlpart
integration, but I think it'd convenient to commit this patch now so I
paste the patch here for review. Any bugfix, suggestion or comment on
the patch is welcomed =).

Greetings,
       Eduardo Robles Elvira.

[kwallet-konqi2.patch]

---

Index: khtmlview.cpp
===================================================================
--- khtmlview.cpp (revisión: 943726)
+++ khtmlview.cpp (copia de trabajo)
@@ -3527,6 +3527,20 @@
     cg.sync();
 }
 
+
+void KHTMLView::delNonPasswordStorableSite(const QString& host)
+{
+    if (!d->formCompletions) {
+        d->formCompletions = new KConfig(KStandardDirs::locateLocal("data", "khtml/formcompletions"));
+    }
+
+    KConfigGroup cg( d->formCompletions, "NonPasswordStorableSites");
+    QStringList sites = cg.readEntry("Sites", QStringList());
+    sites.removeOne(host);
+    cg.writeEntry("Sites", sites);
+    cg.sync();
+}
+
 bool KHTMLView::nonPasswordStorableSite(const QString& host) const
 {
     if (!d->formCompletions) {
Index: khtml_part.h
===================================================================
--- khtml_part.h (revisión: 943726)
+++ khtml_part.h (copia de trabajo)
@@ -244,6 +244,7 @@
   friend class KHTMLPartFunction;
   friend class KHTMLPopupGUIClient;
   friend class KHTMLFind;
+  friend class StorePass;
   friend class WebCore::SVGDocumentExtensions;
 
   Q_PROPERTY( bool javaScriptEnabled READ jScriptEnabled WRITE setJScriptEnabled )
@@ -1480,6 +1481,9 @@
   void slotWalletClosed();
   void launchWalletManager();
   void walletMenu();
+  void delNonPasswordStorableSite();
+  void removeStoredPasswordForm(QAction* action);
+  void addWalletFormKey(const QString& walletFormKey);
 
   /**
    * @internal
@@ -1664,6 +1668,7 @@
   void openWallet(DOM::HTMLFormElementImpl*);
   void saveToWallet(const QString& key, const QMap<QString,QString>& data);
   void dequeueWallet(DOM::HTMLFormElementImpl*);
+  void saveLoginInformation(const QString& host, const QString& key, QMap<QString, QString>& walletMap);
 
   void enableFindAheadActions(bool);
 
Index: khtmlview.h
===================================================================
--- khtmlview.h (revisión: 943726)
+++ khtmlview.h (copia de trabajo)
@@ -103,6 +103,7 @@
     friend class DOM::DocumentImpl;
     friend class KHTMLPart;
     friend class KHTMLFind;
+    friend class StorePass;
     friend class khtml::RenderCanvas;
     friend class khtml::RenderObject;
     friend class khtml::RenderLineEdit;
@@ -472,6 +473,7 @@
     void addFormCompletionItem(const QString &name, const QString &value);
 
     void addNonPasswordStorableSite( const QString& host );
+    void delNonPasswordStorableSite( const QString& host );
     bool nonPasswordStorableSite( const QString& host ) const;
 
     bool dispatchMouseEvent(int eventId, DOM::NodeImpl *targetNode,
Index: khtmlpart_p.h
===================================================================
--- khtmlpart_p.h (revisión: 943726)
+++ khtmlpart_p.h (copia de trabajo)
@@ -53,6 +53,7 @@
 #include "xml/dom_nodeimpl.h"
 #include "editing/editing_p.h"
 #include "find/khtmlfind_p.h"
+#include "html/storepassbar.h"
 
 class KFind;
 class KFindDialog;
@@ -145,7 +146,7 @@
   KHTMLPartPrivate& operator=(const KHTMLPartPrivate&);
 public:
   KHTMLPartPrivate(KHTMLPart* part, QObject* parent) :
-    m_find( part )
+    m_find( part ), m_storePass( part )
   {
     q     = part;
     m_doc = 0L;
@@ -293,6 +294,7 @@
 
 #ifndef KHTML_NO_WALLET
   KWallet::Wallet* m_wallet;
+  QStringList m_walletForms;
 #endif
   int m_runningScripts;
   bool m_bOpenMiddleClick;
@@ -447,6 +449,7 @@
   QStringList m_pluginPageQuestionAsked;
 
   KHTMLFind m_find;
+  StorePass m_storePass;
 
   KJSErrorDlg *m_jsedlg;
 
Index: khtml_part.cpp
===================================================================
--- khtml_part.cpp (revisión: 943726)
+++ khtml_part.cpp (copia de trabajo)
@@ -62,6 +62,7 @@
 
 #include "khtmlview.h"
 #include <kparts/partmanager.h>
+#include <kacceleratormanager.h>
 #include "ecma/kjs_proxy.h"
 #include "ecma/kjs_window.h"
 #include "khtml_settings.h"
@@ -681,6 +682,11 @@
 {
   kDebug( 6050 ) << this << "opening" << url;
 
+  // Wallet forms are per page, so clear it when loading a different page if we
+  // are not an iframe (because we store walletforms only on the topmost part).
+  if(!parentPart())
+    d->m_walletForms.clear();
+  
   d->m_redirectionTimer.stop();
 
   // check to see if this is an "error://" URL. This is caused when an error
@@ -6961,7 +6967,7 @@
   d->m_wallet = wallet;
   d->m_bWalletOpened = true;
   connect(d->m_wallet, SIGNAL(walletClosed()), SLOT(slotWalletClosed()));
-
+  d->m_walletForms.clear();
   if (!d->m_statusBarWalletLabel) {
     d->m_statusBarWalletLabel = new KUrlLabel(d->m_statusBarExtension->statusBar());
     d->m_statusBarWalletLabel->setFixedHeight(KHTMLGlobal::iconLoader()->currentSize(KIconLoader::Small));
@@ -7028,12 +7034,85 @@
 void KHTMLPart::walletMenu()
 {
 #ifndef KHTML_NO_WALLET
-  KMenu *m = new KMenu(0L);
-  m->addAction(i18n("&Close Wallet"), this, SLOT(slotWalletClosed()));
-  m->popup(QCursor::pos());
+  KMenu *menu = new KMenu(0L);
+  QActionGroup *menuActionGroup = new QActionGroup(menu);
+  connect( menuActionGroup, SIGNAL(triggered(QAction*)), this, SLOT(removeStoredPasswordForm(QAction*)) );
+  
+  menu->addAction(i18n("&Close Wallet"), this, SLOT(slotWalletClosed()));
+  
+  if (d->m_view && d->m_view->nonPasswordStorableSite(toplevelURL().host())) {
+    menu->addAction(i18n("&Allow storing passwords for this site"), this, SLOT(delNonPasswordStorableSite()));
+  }
+  
+  // List currently removable form paswords
+  for ( QStringList::Iterator it = d->m_walletForms.begin(); it != d->m_walletForms.end(); ++it ) {
+      QAction* action = menu->addAction( i18n("Remove password for form %1", *it) );
+      action->setActionGroup(menuActionGroup);
+      QVariant var(*it);
+      action->setData(var);
+  }
+  
+  KAcceleratorManager::manage(menu);
+  menu->popup(QCursor::pos());
 #endif // KHTML_NO_WALLET
 }
 
+void KHTMLPart::removeStoredPasswordForm(QAction* action)
+{
+#ifndef KHTML_NO_WALLET
+  assert(action);
+  assert(d->m_wallet);
+  QVariant var(action->data());
+  
+  if(var.isNull() || !var.isValid() || var.type() != QVariant::String)
+    return;
+  
+  QString key = var.toString();
+  if (KWallet::Wallet::keyDoesNotExist(KWallet::Wallet::NetworkWallet(),
+                                      KWallet::Wallet::FormDataFolder(),
+                                      key))
+    return; // failed
+  
+  
+  if (!d->m_wallet->hasFolder(KWallet::Wallet::FormDataFolder()))
+    return; // failed
+  
+  d->m_wallet->setFolder(KWallet::Wallet::FormDataFolder());
+  if (d->m_wallet->removeEntry(key))
+    return; // failed
+
+  d->m_walletForms.removeOne(key);
+#endif // KHTML_NO_WALLET
+}
+
+void KHTMLPart::addWalletFormKey(const QString& walletFormKey)
+{
+#ifndef KHTML_NO_WALLET
+  
+  if (parentPart()) {
+    parentPart()->addWalletFormKey(walletFormKey);
+    return;
+  }
+  
+  if(!d->m_walletForms.contains(walletFormKey))
+    d->m_walletForms.append(walletFormKey);
+#endif // KHTML_NO_WALLET
+}
+
+void KHTMLPart::delNonPasswordStorableSite()
+{
+#ifndef KHTML_NO_WALLET
+  if (d->m_view)
+    d->m_view->delNonPasswordStorableSite(toplevelURL().host());
+#endif // KHTML_NO_WALLET
+}
+void KHTMLPart::saveLoginInformation(const QString& host, const QString& key, QMap<QString, QString>& walletMap)
+{
+#ifndef KHTML_NO_WALLET
+  d->m_storePass.saveLoginInformation(host, key, walletMap);
+#endif // KHTML_NO_WALLET
+}
+
 void KHTMLPart::slotToggleCaretMode()
 {
   setCaretMode(d->m_paToggleCaretMode->isChecked());
Index: html/storepassbar.cpp
===================================================================
--- html/storepassbar.cpp (revisión: 0)
+++ html/storepassbar.cpp (revisión: 0)
@@ -0,0 +1,115 @@
+/* This file is part of the KDE project
+ *
+ * Copyright (C) 2009 Eduardo Robles Elvira <edulix at gmail dot com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#include "storepassbar.h"
+
+#include "khtmlviewbar.h"
+#include "khtml_part.h"
+#include "khtmlview.h"
+#include <kcolorscheme.h>
+#include <QPalette>
+
+StorePassBar::StorePassBar( QWidget *parent ) :
+  KHTMLViewBarWidget( true, parent )
+{
+  setupUi( centralWidget() );
+
+  m_store->setIcon( KIcon( "dialog-ok" ) );
+  // Same as KStandardGuiItem::no()
+  m_neverForThisSite->setIcon( KIcon( "process-stop" ) );
+  m_doNotStore->setIcon( KIcon( "dialog-cancel" ) );
+  centralWidget()->setFocusProxy( m_store );
+  
+  QPalette pal = palette();
+  KColorScheme::adjustBackground(pal, KColorScheme::ActiveBackground);
+  setPalette(pal);
+  setBackgroundRole(QPalette::Base);
+  setAutoFillBackground(true);
+
+  connect( m_store, SIGNAL(clicked()), this, SIGNAL(storeClicked()) );
+  connect( m_neverForThisSite, SIGNAL(clicked()), this,
+    SIGNAL(neverForThisSiteClicked()) );
+  connect( m_doNotStore, SIGNAL(clicked()), this,
+    SIGNAL(doNotStoreClicked()) );
+
+  m_store->setFocus();
+}
+
+
+void StorePassBar::setHost(const QString& host)
+{
+  if(host.isEmpty())
+    m_label->setText( i18n("Do you want to store this password?") );
+  else
+    m_label->setText( i18n("Do you want to store this password for %1?", host) );
+}
+
+
+
+StorePass::StorePass( KHTMLPart *part ) :
+  m_part( part )
+{
+  connect( &m_storePassBar, SIGNAL(storeClicked()), this, SLOT(slotStoreClicked()) );
+  connect( &m_storePassBar, SIGNAL(neverForThisSiteClicked()), this,
+    SLOT(slotNeverForThisSiteClicked()) );
+  connect( &m_storePassBar, SIGNAL(doNotStoreClicked()), this,
+    SLOT(slotDoNotStoreClicked()) );
+}
+
+StorePass::~StorePass()
+{
+}
+
+void StorePass::saveLoginInformation(const QString& host, const QString& key,
+  QMap<QString, QString>& walletMap)
+{
+  m_host = host;
+  m_key = key;
+  m_walletMap = walletMap;
+  m_storePassBar.setHost(host);
+  
+  m_part->pTopViewBar()->addBarWidget( &m_storePassBar );
+  m_part->pTopViewBar()->showBarWidget( &m_storePassBar );
+}
+
+void StorePass::removeBar()
+{
+  m_part->pTopViewBar()->hideCurrentBarWidget();
+  m_walletMap.clear();
+  m_host = m_key = "";
+  m_storePassBar.setHost(m_host);
+}
+
+void StorePass::slotStoreClicked()
+{
+  m_part->saveToWallet(m_key, m_walletMap);
+  removeBar();
+}
+
+void StorePass::slotNeverForThisSiteClicked()
+{
+  m_part->view()->addNonPasswordStorableSite(m_host);
+  removeBar();
+}
+    
+void StorePass::slotDoNotStoreClicked()
+{
+  removeBar();
+}
Index: html/html_formimpl.cpp
===================================================================
--- html/html_formimpl.cpp (revisión: 943726)
+++ html/html_formimpl.cpp (copia de trabajo)
@@ -487,6 +487,10 @@
     if (w->readMap(key, map))
         return; // failed, abort
 
+    if(document()->view())
+    {
+        document()->view()->part()->addWalletFormKey(key);
+    }
     for (QListIterator<HTMLGenericFormElementImpl*> it(formElements); it.hasNext();) {
         HTMLGenericFormElementImpl* const cur = it.next();
         if (cur->id() == ID_INPUT) {
@@ -644,42 +648,8 @@
             }
 
             if ( doesnotexist || !w || login_changed ) {
-                // TODO use KMessageBox::questionYesNoCancel() again, if you can pass a KGuiItem for Cancel
-                KDialog* const dialog = new KDialog();
-                dialog->setObjectName( "questionYesNoCancel" );
-                dialog->setCaption( i18n("Save Login Information") );
-                dialog->setButtons( KDialog::Yes | KDialog::No | KDialog::Cancel | KDialog::Details);
-                dialog->setButtonGuiItem( KDialog::Yes, KGuiItem(i18n("Store")) );
-                dialog->setButtonGuiItem( KDialog::No, KGuiItem(i18n("Ne&ver for This Site")) );
-                dialog->setButtonGuiItem( KDialog::Cancel, KGuiItem(i18n("Do Not Store")) );
-                dialog->setDefaultButton( KDialog::Yes );
-                dialog->setEscapeButton( KDialog::Cancel );
-                dialog->setModal( true );
-                dialog->showButtonSeparator( true );
-
-                bool checkboxResult = false;
-                const int savePassword = KMessageBox::createKMessageBox(dialog, QMessageBox::Information,
-                                                                            formUrl.host().isEmpty() ? // e.g. local file
-                                                                            i18n("Do you want to store this password?") :
-                                                                            i18n("Do you want to store this password for %1?",
-                                                                                 formUrl.host()),
-                                                                            QStringList(), QString(), &checkboxResult, KMessageBox::Notify,
-                                                                            i18n("Konqueror has the ability to store the password "
-                                                                                 "in an encrypted wallet. When the wallet is unlocked, it "
-                                                                                 "can then automatically restore the login information "
-                                                                                 "next time you submit this form."));
-
-                if ( savePassword == KDialog::Yes ) {
-                    // ensure that we have the user / password inside the url
-                    // otherwise we might have a potential security problem
-                    // by saving passwords under wrong lookup key.
-
-                    if (view->part()) {
-                        view->part()->saveToWallet(key, m_walletMap);
-                    }
-                } else if ( savePassword == KDialog::No ) {
-                    view->addNonPasswordStorableSite(formUrl.host());
-                }
+                if (view->part())
+                    view->part()->saveLoginInformation(formUrl.host(), key, m_walletMap);
             }
         }
 #endif // KHTML_NO_WALLET
Index: html/storepassbar_base.ui
===================================================================
--- html/storepassbar_base.ui (revisión: 0)
+++ html/storepassbar_base.ui (revisión: 0)
@@ -0,0 +1,98 @@
+<ui version="4.0" >
+ <class>StorePassBarBase</class>
+ <widget class="QWidget" name="StorePassBarBase" >
+  <property name="geometry" >
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>640</width>
+    <height>32</height>
+   </rect>
+  </property>
+  <property name="sizePolicy" >
+   <sizepolicy vsizetype="Minimum" hsizetype="Maximum" >
+    <horstretch>0</horstretch>
+    <verstretch>0</verstretch>
+   </sizepolicy>
+  </property>
+  <property name="windowTitle" >
+   <string/>
+  </property>
+  <layout class="QGridLayout" name="gridLayout" >
+   <property name="margin" >
+    <number>0</number>
+   </property>
+   <property name="spacing" >
+    <number>0</number>
+   </property>
+   <item row="0" column="0" >
+    <layout class="QHBoxLayout" name="horizontalLayout" >
+     <property name="spacing" >
+      <number>4</number>
+     </property>
+     <item>
+      <widget class="QLabel" name="m_label" >
+       <property name="sizePolicy" >
+        <sizepolicy vsizetype="Preferred" hsizetype="Maximum" >
+         <horstretch>0</horstretch>
+         <verstretch>0</verstretch>
+        </sizepolicy>
+       </property>
+       <property name="text" >
+        <string>Do you want to store this password?</string>
+       </property>
+       <property name="buddy" >
+        <cstring>m_find</cstring>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <spacer name="horizontalSpacer" >
+       <property name="orientation" >
+        <enum>Qt::Horizontal</enum>
+       </property>
+       <property name="sizeHint" stdset="0" >
+        <size>
+         <width>40</width>
+         <height>20</height>
+        </size>
+       </property>
+      </spacer>
+     </item>
+     <item>
+      <widget class="QToolButton" name="m_store" >
+       <property name="text" >
+        <string>&Store</string>
+       </property>
+       <property name="toolButtonStyle" >
+        <enum>Qt::ToolButtonTextBesideIcon</enum>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <widget class="QToolButton" name="m_neverForThisSite" >
+       <property name="text" >
+        <string>Ne&ver for This Site</string>
+       </property>
+       <property name="toolButtonStyle" >
+        <enum>Qt::ToolButtonTextBesideIcon</enum>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <widget class="QToolButton" name="m_doNotStore" >
+       <property name="text" >
+        <string>Do &Not Store</string>
+       </property>
+       <property name="toolButtonStyle" >
+        <enum>Qt::ToolButtonTextBesideIcon</enum>
+       </property>
+      </widget>
+     </item>
+    </layout>
+   </item>
+  </layout>
+ </widget>
+ <resources/>
+ <connections/>
+</ui>
Index: html/storepassbar.h
===================================================================
--- html/storepassbar.h (revisión: 0)
+++ html/storepassbar.h (revisión: 0)
@@ -0,0 +1,69 @@
+/* This file is part of the KDE project
+ *
+ * Copyright (C) 2009 Eduardo Robles Elvira <edulix at gmail dot com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+#ifndef __storepassbar_h__
+#define __storepassbar_h__
+
+#include "khtmlviewbarwidget.h"
+#include "ui_storepassbar_base.h"
+
+#include <QObject>
+#include <QMap>
+
+class KHTMLPart;
+
+class StorePassBar : public KHTMLViewBarWidget, private Ui::StorePassBarBase
+{
+  Q_OBJECT
+public:
+  StorePassBar( QWidget *parent = 0 );
+    
+  void setHost(const QString& host);
+    
+signals:
+  void storeClicked();
+  void neverForThisSiteClicked();
+  void doNotStoreClicked();
+};
+
+class StorePass : public QObject
+{
+  Q_OBJECT
+public:
+  StorePass( KHTMLPart *part );
+  ~StorePass();
+    
+  void saveLoginInformation(const QString& host, const QString& key,
+    QMap<QString, QString>& walletMap);
+  void removeBar();
+
+private slots:
+  void slotStoreClicked();
+  void slotNeverForThisSiteClicked();
+  void slotDoNotStoreClicked();
+  
+private:
+  KHTMLPart *m_part;
+  
+  StorePassBar m_storePassBar;
+  QString m_host;
+  QString m_key;
+  QMap<QString, QString> m_walletMap;
+};
+#endif
Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt (revisión: 943726)
+++ CMakeLists.txt (copia de trabajo)
@@ -247,6 +247,7 @@
   ${CMAKE_SOURCE_DIR}/khtml/html/html_objectimpl.cpp
   ${CMAKE_SOURCE_DIR}/khtml/html/html_tableimpl.cpp
   ${CMAKE_SOURCE_DIR}/khtml/html/html_canvasimpl.cpp
+  ${CMAKE_SOURCE_DIR}/khtml/html/storepassbar.cpp
   ${CMAKE_SOURCE_DIR}/khtml/html/HTMLMediaElement.cpp
   ${CMAKE_SOURCE_DIR}/khtml/html/HTMLAudioElement.cpp
   ${CMAKE_SOURCE_DIR}/khtml/html/HTMLVideoElement.cpp
@@ -254,6 +255,10 @@
   ${CMAKE_SOURCE_DIR}/khtml/html/TimeRanges.cpp
 )
 
+kde4_add_ui_files(khtmlhtml_STAT_SRCS
+  ${CMAKE_SOURCE_DIR}/khtml/html/storepassbar_base.ui
+)
+
 # khtml/ecma/Makefile.am: kjs_html
 
 set(kjs_html_STAT_SRCS

---