> On Sept. 29, 2012, 6:48 a.m., David Faure wrote:
> > Doesn't this break "moving a tab with MMB" then? The KTabBar
> > mousePress/mouseRelease code won't be called anymore.
> >
> > If this is the case, then what we really have is two incompatible
> > features... so we could just remove the "moving with MMB" code in ktabbar
> > and this eventFilter wouldn't be needed.
>
> Dawit Alemayehu wrote:
> Why remove a feature other application might want to have or was the
> "moving a tab with MMB" specifically added for Konqueror's sake ? Anyhow,
> this patch does not really break "moving a tab with MMB" unless the user
> checks the "Middle-click on a tab to close it" option. Otherwise, the MMB
> click on a tab will continue to behave as it does now.
>
> David Faure wrote:
> You didn't reply as to whether this breaks "moving a tab with MMB". If it
> does, that's fine with me, given that "moving a tab with LMB" works and is
> much more intuitive anyway.
>
> My point is the following: this MMB-move in KTabBar is a compatibility
> feature only. I would like to see it removed in KF5, for more compatibility
> with QTabBar (possibly so that KTabBar isn't even needed anymore). At that
> point, the eventFilter in this commit (which is a workaround for that
> MMB-sends-LMB hack in KTabBar) will just be overkill, the old code would be
> fine again. Will we remember to revert this commit then? OK here's a
> solution: please add a #pragma message("KF5: revert the commit that
> introduced this line") in your patch, inside #ifdef QT_VERSION >=
> QT_VERSION_CHECK(5, 0, 0), so that we remember doing it. I don't like keeping
> complicated code for the long run, it only makes maintainance harder.
I thought I did. Let me rephrase it. Moving a tab with MMB works so long as the
user does not explicitly check the "Middle-click on a tab to close it" option.
If the user enables that option then clicking with the MMB will result in the
tab being closed without the side effects that is currently being observed,
i.e. the current active tab being changed. So yes, moving with MMB will work
fine with this patch.
I understand what you mean. If the issue is fixed in KTabBar or KTabBar no
longer becomes necessary, then this fix is unnecessary and as such it should be
removed. Hence, I will add the #pragma message as you suggest and push the fix
upstream.
- Dawit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106615/#review19560
-----------------------------------------------------------
On Oct. 8, 2012, 5:26 p.m., Dawit Alemayehu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106615/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2012, 5:26 p.m.)
>
>
> Review request for KDE Base Apps and David Faure.
>
>
> Description
> -------
>
> The attached patch fixes the bug where clicking on a tab with the MMB while
> the "Middle-click on a tab to close it" option is checked results in the tab
> gaining the focus first before being closed instead of being closed in the
> background. That results in the active tab being changed when a user
> activates the aforementioned option and clicks on a background tab with the
> MMB.
>
> For the record the reason why this really happens is KTabBar's treatment of
> the MMB clicks when the setMovable is set to TRUE. In addition to emitting a
> mouseMiddleClick signal which Konqueror currently relies on to close the tab,
> it also eats the MMB click event and generates a new LMB click event in its
> place. Apparently that was done for compatibility sake so that the user can
> move tabs using either the LMB or MMB.
>
> Anyhow, this patch addresses the problem by installing an event filter to
> intercept the mouse events from the tabbar and handle MMB clicks on its own
> when the aforementioned option is checked so that we do not receive
> unnecessary bogus events.
>
>
> This addresses bug 264058.
> http://bugs.kde.org/show_bug.cgi?id=264058
>
>
> Diffs
> -----
>
> konqueror/src/konqtabs.h 4b6f1f1
> konqueror/src/konqtabs.cpp 611659f
>
> Diff: http://git.reviewboard.kde.org/r/106615/diff/
>
>
> Testing
> -------
>
> Check/uncheck the "Middle-click on a tab to close it" option and tested the
> behavior.
>
> - When the option is unchecked, clicking on a background tab with MMB behaves
> the same way as it currently does.
> - When the option is checked, clicking on a background tab with MMB will
> immediately close it without changing the current active tab.
>
>
> Thanks,
>
> Dawit Alemayehu
>
>
