Middle Clicking on BookmarkBar and BookmarkMenu entries

[Daniel Teske]

>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?
>  
>
I'm not sure, what would be usefull to other applications. I think there 
needs to be a keyboard equivalent to middle clicking. Ctrl+Enter?

>* 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?
>  
>
Well, I looked into it, and I intended to store it inside 
KToolbarButtonPrivate, and KAction asking KToolbarButton which button 
was presed.
Basically inside KAction::slotActivated(), roughly:

if(::qt_cast<KToolBarButton *>( senderObj ))
    d->state = senderObj->state();

I was pretty sure that there was a way to add it for KPopupMenu, but 
looking at it again, I don't remember how.

>>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().
>  
>
slotBookmarkSelected assumes that it is only used by its 
KBookmarkbar/Menu. As it is public, you do have a point, I'll fix that.

>+    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.
>  
>
I don't understand what you mean by dangling. That const reference is 
perfectly safe, it avoids the cost of one copy constructor.
(Which for implicit shared is smaller than I thought.)