Summary: | Additional items are selected when using "Forward" and "Back" mouse buttons | ||
---|---|---|---|
Product: | [Applications] dolphin | Reporter: | David Rosca <nowrep> |
Component: | view-engine: general | Assignee: | Dolphin Bug Assignee <dolphin-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | emmanuelpescosta099 |
Priority: | NOR | ||
Version: | 4.11.3 | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/kde-baseapps/41ece8e93da5058a14b2431d730f055a0653b44d | Version Fixed In: | 4.11.4 |
Attachments: |
Don't propagate those events
Screenshot describing the issue |
Description
David Rosca
2013-11-10 14:33:15 UTC
Created attachment 83475 [details]
Don't propagate those events
Created attachment 83476 [details]
Screenshot describing the issue
Thanks for the bug report + patch!!
> if (event->buttons() & (Qt::XButton1 | Qt::XButton2) && m_pressedIndex < 0) {
Why do you check for m_pressedIndex < 0 here?
This means:
When you press the back or forward button above an item, the code in your if statement will not be called, because the m_pressedIndex is equal or greater than 0 (and not less than 0), so the item will be selected instead.
You can simply fix this behavior, by removing the m_pressedIndex < 0 from the if condition.
Btw. you can move this button check before "m_pressedMousePos = transform.map(event->pos());" and add "emit mouseButtonPressed(-1, event->buttons());" to the if statement before "return true".
The -1 means pressed above the viewport.
> When you press the back or forward button above an item, the code in your if statement will not be called, because the m_pressedIndex is equal or greater than 0 (and not less than 0), so the item will be selected instead.
This is correct, I just copied the check (for m_pressedIndex) from src/views/dolphinview.cpp DolphinView::slotMouseButtonPressed.
There is a comment explaining that pressing those buttons on items should just focus them.
"// Trigger the history navigation only when clicking on the viewport:
// Above an item the XButtons provide a simple way to select items in
// the singleClick mode."
Actually, the whole idea of this patch is simply ignore the mouse press when going back / forward in history.
> There is a comment explaining that pressing those buttons on items should just focus them.
Oh sorry, I didn't know that.
In my opinion, we should change this behavior to the default back/forward mouse button behavior: always trigger the history navigation on back/forward button click.
This is what the user expects from these buttons.
@Frank, @you
What do you think?
> In my opinion, we should change this behavior to the default back/forward mouse button behavior: always trigger the history navigation on back/forward button click. There already is a bug exactly for this: https://bugs.kde.org/show_bug.cgi?id=310288 For me, I don't mind it to be either way (I get used to press those buttons only outside items, so it won't make difference). The patch looks good to me - thanks! Maybe a comment wouldn't hurt though, to explain the point of the m_pressedIndex check (and that it should be revisited if the "select items when pressing bakc/forward button" behavior is ever changed). Do you have a git account, or shall we push the commit for you? (In reply to comment #5) > @Frank, @you > What do you think? Well, I don't have such a mouse and have never used one, so I'm probably not the best person to ask ;-) But anyway, on the one hand, I understand that the point of this behavior is to make selecting items in single-click mode more convenient. On the other hand, we already have the selection markers, Ctrl+click, and rubberband selection for that, and I'm not sure if we really need one more way to select items, in particular if we consider that quite a few users expect that the back/forward buttons are only used for navigating back/forward, which is understandable. In any case, any additional comments about this issue should be added to bug 310288. > Do you have a git account, or shall we push the commit for you?
I don't have account at KDE git, can you please push it for me? Thanks
Git commit 41ece8e93da5058a14b2431d730f055a0653b44d by Frank Reininghaus, on behalf of David Rosca. Committed on 14/11/2013 at 08:08. Pushed by freininghaus into branch 'KDE/4.11'. Do not select items when navigating back/forward with the mouse If we detect that the user pressed the back/forward buttons while hovering the empty space of the view, such that DolphinView::slotMouseButtonPressed(int, Qt::MouseButtons) will request that Dolphin navigates back/forward in the history, handling the mouse press event should stop. This prevents the possible unexpected selection of items in the new directory. FIXED-IN: 4.11.4 M +8 -0 dolphin/src/kitemviews/kitemlistcontroller.cpp http://commits.kde.org/kde-baseapps/41ece8e93da5058a14b2431d730f055a0653b44d |