Middle Clicking on BookmarkBar and BookmarkMenu entries

[Daniel Teske]

Hi

This is a (not quite finished patch) to add middle clicking to the
bookmark bar and bookmark menu.
The Bug 33439 [1]: middle mouse button click on toolbars has 111[2]
votes, which makes it the most voted for bug in konqueror/bookmarks.

As the last patches to kdelibs/kio/bookmarks are 10 month old, I'll try
to explain all the details.

I'll start with an overview of the classes:

The bookmarkbar is a normal KToolbar, constructed by konqueror.
KBookmarkBar wraps around the KToolbar and adds the bookmarks
(KToolbarButtons) and provides an abstraction.
Similar the top level menu is constructed by konqueror, and
KBookmarkMenu wraps around it.
Any submenues are constructed by KBookmarkBar/Menu.
Konqueror provides KBookmarkBar/Menu with an KBookmarkOwner, which is
used to communicate.

The two basic problems are:
a) Differentiate between a left click and a middle click.
b) Inform konqueror whether to open the bookmark in a new tab.

a) The basic problem is: Neither KPopupMenu nor KToolbarButton provide a
method to identify the user interaction, which causes the activated() or
clicked() signal.

The attached kdeui-patch adds a method state() to KPopupMenu and
KToolbarButton, which provides additional information to the receiver of
the signal. 
(Note: Changes to KPopupMenu and KToolbarButton can be avoided, I
outline how further down.)

state() returns a Qt::Buttonstate (Bitmask of Mouse Buttons and Modifier
Keys).

QPopupMenu can emit a activated() signal as the result of calling:
MouseReleaseEvent(), KeyPressEvent() or activateItemAt().
This patch redefines all of them, to save how the user caused the
activate() signal to be emitted.

QToolButton can emit a clicked() signal in response to calling:
MouseReleaseEvent(), keyPressEvent(), keyReleaseEvent(),
activateItemAt(), autoRepeatTimeOut(), animateClick() and
emulateClick().

The basic idea is the same: Save the state before a clicked() signal
might be emitted, albeit with some additional magic:

The easy ones:
keyPressEvent():
Excatly like KPopupMenu::keyPressEvent()

keyReleaseEvent():
Similar to keyPressEvent()

emulateClick():
is a private slot of QButton, which is not called inside QButton.
As it seems to be unused, it won't emit a signal.

animateClick():
Well, there is no way to modifier the state variable. So it will
contain an undefined value. But as this function isn't called in
response to a user event, but by the application, it probably isn't
a big problem

The hard ones:
autoRepeatTimeOut():
No way to save the correct state. So state() won't reliably work for
KToolbarButton's with enabled autoRepeat

MouseReleaseEvent():
Unfortunately QToolButton doesn't emit a clicked() signal in response
to a middle click. So we emulate a left click, at the same position.
This means MousePressEvent() must be modified as well.

Currently KBookmarkBar/Menu directly ask the KToolbar/KPopupMenu whether
the middle button was used. This obviously isn't perfect, because
ideally KAction would have state() method. 
(That is doable, but I have kept the patch as small as is resonable.)

I don't know wheter to differentiate between left and middle click might
be usefull to other applications.

There are two other ways to fix the bug, which don't require changes to
KPopupMenu and KToolbarButton and limit the changes to konqueror and
kio/bookmarks.


1) Derive new classes KBookmarkPopupMenu and KBookmarkToolbarButton,
which redefine the methods and use those new classes.
Due to the way the KPopups are constructed, that would entail a small
change to KActionMenu. (A way to replace the KActionMenu::d->popupMenu
in derived classes.) While doable, it doesn't feel right.

2) Install event filters at the right places. This would be relatively
easy, and I think the best solution.

So what should I do?
a) Add KAction::state()
b) Derived classes
c) event filters

Unto problem 2)
Letting konqueror know, wheter to open a new tab.
Because I'm not sure how to do that cleanly, or rather without
increasing the mess. So I'll enumerate the ways konqueror interacts with
KBookmarkBar and KBookmarkMenu:
Well, I already mentioned that both KBookmarkBar and KBookmarkMenu
constructos take a pointer to a KBookmarkOwner (or a
KExtendtedBookmarkOwner). 

KBookmarkOwner is a abstract class, with the following functions:
virtual void openBookmarkURL (const QString &_url)
virtual QString currentTitle () const
virtual QString currentURL () const 

Looks reasonable so far.

KExtendedBookmarkOwner derived from KBookmarkOwner and adds to the
interface:
Slots:
  void fillBookmarksList (KExtendedBookmarkOwner::QStringPairList &list)
Signals:
  void signalFillBookmarksList (KExtendedBookmarkOwner::QStringPairList 
&list)
These functions are needed for "Bookmark Tabs as folder",
KBookmarkBar/Menu calls fillBookmarksList, which emits the signal.
Konqueror receives the signal and fills the list.

And to complete it:
Both KBookmarkBar and KBookmarkMenu emit the signal:
void aboutToShowContextMenu (const KBookmark &, QPopupMenu *)
if the user right clicks on a bookmark entry.

For this patch I added a signal directly to KBookmarkBar and
KBookmarkMenu. The other reasonable place would to add it to
KExtendedBookmarkOwner.

daniel teske

[1] http://bugs.kde.org/show_bug.cgi?id=33439
[2] Alaaf! ;-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdeui-patch
Type: text/x-patch
Size: 6005 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20041213/a88fff96/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kio-bookmarks-patch
Type: text/x-patch
Size: 5918 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20041213/a88fff96/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: konqueror-patch
Type: text/x-patch
Size: 3156 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20041213/a88fff96/attachment-0002.bin>