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

List:       kde-core-devel
Subject:    Re: patch: khtml
From:       David Faure <david () mandrakesoft ! com>
Date:       2002-03-17 22:51:56
[Download RAW message or body]

On Monday 18 March 2002 00:00, Simon Hausmann wrote:
> On Sun, Mar 17, 2002 at 09:42:25PM +0100, David Faure wrote:
> > On Sunday 17 March 2002 10:41, Simon Hausmann wrote:
> > > Hi,
> > > 
> > > The following bug is driving me mad :)
> > > 
> > > Load any web site and then select the complete URL in the locationbar.
> > > Now the 'copy' action is disabled. (but why did I select al the text? Yes, to
> > > copy it :)
> > > 
> > > The attached patch fixes it for me. I'm not 100% it's the cleanest fix.
> > > Another approach would be to change konqueror to disallow khtml changing
> > > the state of the copy action, in ::slotEnableAction. That would require
> > > hardcoding a list of actions that may not be changed by the part when
> > > the lineedit in the locationbar has the focus -- can't disconnect from
> > > enableAction completely because for other actions it makes sense to
> > > enable/disable even if the locationbar has the focus.
> > 
> > I see the problem. khtml loses its selection (recent change), and disables
> > "Copy" because of that.
> > But the fix looks wrong - when khtml gets the focus back, the action state will be wrong.
> > And letting the parts handle this makes it hard to write parts, which is against the KParts
> > concept ;)  IMHO the fix isn't in the part, but in konqueror.
> > It could very well prevent any part from touching cut/copy/paste while the
> > location bar has focus. After a quick look at the code, this sounds like simply
> > testing for KonqMainWindow's m_bLocationBarConnected, from KonqView::slotEnableAction
> > 
> > Hmm, it should also update m_bCopyWasEnabled etc. Damn, this is old
> > stuff, we could use the browser-extension's action status bitfield instead of
> > all those bools...

Just tested the patch... it crashes if the part has no browser extension...
if (ext) is easy to add, but this means the action states will not be restored
if there is no extension.... Hmm, I guess they should all be disabled, those
actions aren't implemented if there's no extension.

> > Ok, enough talk, let's make a patch. Please test the attached one ;)
> 
> Looks good. Just a thought: Why not move the test for those actions
> along with the test whether the locationbar is connected or not
> right into KonqMainWindow::enableAction?

Good idea.
Both things done in attached patch.

-- 
David FAURE, david@mandrakesoft.com, faure@kde.org
http://people.mandrakesoft.com/~david/, http://www.konqueror.org/
KDE, Making The Future of Computing Available Today


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

? konq_
? konq_mainwindow.cc.2
? konqueror.diff_v1
? keditbookmarks/ie_favourites
? sidebar/trees/dirtree.diff
Index: konq_mainwindow.cc
===================================================================
RCS file: /home/kde/kdebase/konqueror/konq_mainwindow.cc,v
retrieving revision 1.967
diff -u -p -r1.967 konq_mainwindow.cc
--- konq_mainwindow.cc	2002/03/15 21:47:10	1.967
+++ konq_mainwindow.cc	2002/03/17 22:51:18
@@ -2383,10 +2383,6 @@ bool KonqMainWindow::eventFilter(QObject
       connect( m_combo->lineEdit(), SIGNAL(textChanged(const QString &)), this, \
                SLOT(slotCheckComboSelection()) );
       connect( m_combo->lineEdit(), SIGNAL(selectionChanged()), this, \
SLOT(slotCheckComboSelection()) );  
-      m_bCutWasEnabled = m_paCut->isEnabled();
-      m_bCopyWasEnabled = m_paCopy->isEnabled();
-      m_bPasteWasEnabled = m_paPaste->isEnabled();
-      m_bDeleteWasEnabled = m_paDelete->isEnabled();
       m_paTrash->setText( i18n("&Delete") ); // Name Delete the action associated \
with 'del'  m_paTrash->setEnabled(true);
       m_paDelete->setEnabled(false);
@@ -2434,12 +2430,22 @@ bool KonqMainWindow::eventFilter(QObject
       disconnect( m_combo->lineEdit(), SIGNAL(textChanged(const QString &)), this, \
                SLOT(slotCheckComboSelection()) );
       disconnect( m_combo->lineEdit(), SIGNAL(selectionChanged()), this, \
SLOT(slotCheckComboSelection()) );  
-      m_paCut->setEnabled( m_bCutWasEnabled );
-      m_paCopy->setEnabled( m_bCopyWasEnabled );
-      m_paPaste->setEnabled( m_bPasteWasEnabled );
-      m_paDelete->setEnabled( m_bDeleteWasEnabled );
-      m_paTrash->setEnabled( m_bDeleteWasEnabled );
-      m_paShred->setEnabled( m_bDeleteWasEnabled );
+      if ( ext ) {
+          m_paCut->setEnabled( ext->isActionEnabled( "cut" ) );
+          m_paCopy->setEnabled( ext->isActionEnabled( "copy" ) );
+          m_paPaste->setEnabled( ext->isActionEnabled( "paste" ) );
+          m_paDelete->setEnabled( ext->isActionEnabled( "delete" ) );
+          m_paTrash->setEnabled( ext->isActionEnabled( "trash" ) );
+          m_paShred->setEnabled( ext->isActionEnabled( "shred" ) );
+      } else {
+          m_paCut->setEnabled( false );
+          m_paCopy->setEnabled( false );
+          m_paPaste->setEnabled( false );
+          m_paDelete->setEnabled( false );
+          m_paTrash->setEnabled( false );
+          m_paShred->setEnabled( false );
+      }
+      m_paTrash->setText( i18n("&Move to Trash") ); // Name back
       m_paTrash->setText( i18n("&Move to Trash") ); // Name back
     }
   }
@@ -3042,6 +3048,10 @@ void KonqMainWindow::enableAction( const
     kdWarning(1202) << "Unknown action " << name << " - can't enable" << endl;
   else
   {
+    if ( m_bLocationBarConnected && (
+      act==m_paCopy || act==m_paCut || act==m_paPaste || act==m_paDelete || \
act==m_paTrash || act==m_paShred ) ) +        // Don't change action state while the \
location bar has focus. +        return;
     //kdDebug(1202) << "KonqMainWindow::enableAction " << name << " " << enabled << \
endl;  act->setEnabled( enabled );
   }
Index: konq_mainwindow.h
===================================================================
RCS file: /home/kde/kdebase/konqueror/konq_mainwindow.h,v
retrieving revision 1.348
diff -u -p -r1.348 konq_mainwindow.h
--- konq_mainwindow.h	2002/03/04 23:58:10	1.348
+++ konq_mainwindow.h	2002/03/17 22:51:18
@@ -513,10 +513,6 @@ private:
 
   KAction *m_ptaFullScreen;
 
-  uint m_bCutWasEnabled:1;
-  uint m_bCopyWasEnabled:1;
-  uint m_bPasteWasEnabled:1;
-  uint m_bDeleteWasEnabled:1;
   uint m_bLocationBarConnected:1;
   uint m_bURLEnterLock:1;
   uint m_bFullScreen:1;



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