Middle Clicking on BookmarkBar and BookmarkMenu entries

David Faure faure at kde.org
Mon Dec 13 09:19:40 GMT 2004


On Monday 13 December 2004 06:44, Daniel Teske wrote:
> So what should I do?
> a) Add KAction::state()
I added a KAction::activationReason() recently, which returns a value out of the enum:
 enum ActivationReason { UnknownActivation, EmulatedActivation, AccelActivation, PopupMenuActivation, ToolBarActivation };
Feel free to replace ToolbarActivation with ToolbarLMBActivation, and add a ToolbarMMBActivation.
(This enum will be new in 3.4, it's still OK to change it).
Maybe we need PopupMenuLMBActivation and PopupMenuRMBActivation too?

KAction::slotActivated sets the reason, but it won't know which mouse button was pressed, unless
* it calls kapp->keyboardMouseState - but I guess that's a tiny race condition, 
if the user released the mouse button already
or
* somehow KToolbarButton "tells" KAction which button was pressed. Well, more likely
the action asks the toolbar button. But adding a new member variable to each 
toolbar button sounds like a bit of a waste; maybe it could be stored into the 
KToolbar instead?

> Unto problem 2)
> For this patch I added a signal directly to KBookmarkBar and
> KBookmarkMenu.
That sounds fine to me. But the patch has a few issues:
dynamic_cast<const KAction *>(sender())->....
Bad idea to use dynamic_cast if not checking the result for 0.
itemId(0) was a bad hack, but it will go away if using KAction::activationReason().

+    const QString & url = sender()->property("url").toString();
Looks dangerous to me - the ref could easily become dangling.
Better use "[const] QString url = ..." without the ref, we have implicit sharing anyway.

> [2] Alaaf! ;-)
What does Alaaf mean? :)

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).




More information about the kfm-devel mailing list