Thanks for the patch.

TEST PLAN
Tested

Please specify what your reviewer has to do to verify your patch is working correctly.

It works everywhere, i use such a mice for years (more than 5) you miss a lot :)

In D14583#302946, @anthonyfieroni wrote:

It works everywhere, i use such a mice for years (more than 5) you miss a lot :)

At least I don't get a MainWindow::mousePressEvent when in Browse mode. Do you know what's making it work for you (somewhere in the stack there must be something handling Qt::ForwardButton)?

Yeah, click in Browse window buttons are not handled, it's needed there ?

In D14583#302948, @anthonyfieroni wrote:

Yeah, click in Browse window buttons are not handled, it's needed there ?

Strange, in your previous comment it was still working fine. As for your question, see D14583#302943.

It's not strange, if you click at main window it works, when you click in browse window it's not, that's normal.

It has a problems with mouse click inside ThumbnailView viewport, when forward/back works, left/right not and opposite.

There are two more problem I noticed, this time in View mode:

• Repeatedly clicking fast on one of the forward or backward buttons will only change the image for every second click.
• Doing the same for videos will accidentally trigger fullscreen mode.

(Thanks to @muhlenpfordt, binding xdotool click 8 / 9 to a keyboard shortcut worked great in uncovering those issues.)

What fullscreen issue? Browse mode should rewritten mouse event handling of thumbnalview that I don't want to bother.

In D14583#304621, @anthonyfieroni wrote:

What fullscreen issue?

See D14583#304542.

Browse mode should rewritten mouse event handling of thumbnalview that I don't want to bother.

Hm, I don't think it's reasonable to expect others to fix the issue for you, and I'm not yet convinced we should land changes which add known bugs, i.e. inconsistent behaviour depending on where you position your mouse.

I found an old MS 5-button mouse at the office, so I can test with real hardware now. :)

In D14583#304542, @rkflx wrote:

There are two more problem I noticed, this time in View mode:

• Repeatedly clicking fast on one of the forward or backward buttons will only change the image for every second click.

I don't think this is fixed. As far as I can see MainWindow::mouseDoubleClickEvent() is never called, only AbstractImageView::mouseDoubleClickEvent() is triggered.

• Doing the same for videos will accidentally trigger fullscreen mode.

I can confirm this. Double clicking the navigation buttons while a video is active (in View Mode) triggers fullscreen.

In Browse Mode the behaviour is strange. Depending on where you click, the selection is moved but immediately cleared or Gwenview switches to View Mode.

In D14583#308392, @muhlenpfordt wrote:

I found an old MS 5-button mouse at the office, so I can test with real hardware now. :)

In D14583#304542, @rkflx wrote:

There are two more problem I noticed, this time in View mode:

• Repeatedly clicking fast on one of the forward or backward buttons will only change the image for every second click.

I don't think this is fixed. As far as I can see MainWindow::mouseDoubleClickEvent() is never called, only AbstractImageView::mouseDoubleClickEvent() is triggered.

I see it's not called, but i can't confirm it changed on second click, it can't do more on this patch.

• Doing the same for videos will accidentally trigger fullscreen mode.

I can confirm this. Double clicking the navigation buttons while a video is active (in View Mode) triggers fullscreen.

So that's not a problem of this patch, you can double click by wheel and it triggered too, it'll by double click on every mouse button.

In Browse Mode the behaviour is strange. Depending on where you click, the selection is moved but immediately cleared or Gwenview switches to View Mode.

I investigate and i don't see how it handled by pos but i'm pretty sure it is, case if pos is over thumb it is selected if not selection is removed, i was pretty sure the problem is mIndexUnderCursor in previewitemdelegate.cpp

What you expect to change on it?

In D14583#327433, @anthonyfieroni wrote:

What you expect to change on it?

IMO the behaviour should be consistent for View and Browse Mode. If this is not feasible maybe it's better to not introduce this feature, sorry.
The main problem I see is the Browse Mode part. It should not depend on where you press the navigation buttons whether this or another action is triggered.

Issues in Browse mode persist without the patch, you can click with Forward/Backward button over image, it's selected.

In D14583#327954, @anthonyfieroni wrote:

Issues in Browse mode persist without the patch, you can click with Forward/Backward button over image, it's selected.

Ok, but I think if Gwenview officially supports these buttons this should work anywhere.

I had a quick look at the Browse Mode problem and tried adding the following event handler:

void ThumbnailView::mousePressEvent(QMouseEvent* event)
{
    if (event->button() == Qt::ForwardButton || event->button() == Qt::BackButton) {
        return;
    }
    QListView::mousePressEvent(event);
}

This does not work in every situation but maybe it's a starting point for you.

• After (left-)clicking an image to view and pressing Return to come back to Browse Mode, pressing a navigation button while over a thumbnail switches to View Mode again
• Subfolders and archives are ignored in navigating by mouse buttons

In D14583#328433, @muhlenpfordt wrote:

This does not work in every situation but maybe it's a starting point for you.

It's material to another patch, see D15612

• After (left-)clicking an image to view and pressing Return to come back to Browse Mode, pressing a navigation button while over a thumbnail switches to View Mode again

I can't reproduce, double click an image pressing return works as pressing key backward or button backward

• Subfolders and archives are ignored in navigating by mouse buttons

Yes, it's normal, you can see actions previous and next are disabled

Ping

Will test again after you land D15612

@ngraham, you can test it.

Just tested all previous mentioned issues on current master:

• "Repeatedly clicking fast on one of the forward or backward buttons will only change the image for every second click"
  • This still exists in View Mode (not in Browse Mode)
• "Doing the same for videos will accidentally trigger fullscreen mode"
  • Still exists if the second of the double clicks happens on the now selected video
• "After (left-)clicking an image to view and pressing Return to come back to Browse Mode, pressing a navigation button while over a thumbnail switches to View Mode again"
  • I still can reproduce this:
    1. Open Gwenview in Browse Mode
    2. Left-click an image -> Switch to View Mode
    3. Press Return to go back to Browse Mode
    4. If you did not move the mouse and it's still over the image which was clicked - if you now press a navigation button the prev/next image is selected and switched to View Mode.

As i mention before, that's not a patch issues.

In D14583#382648, @anthonyfieroni wrote:

As i mention before, that's not a patch issues.

But they become issues when this feature is added. Any chance you could work on fixing them so we can finally land this patch? :)

If someone steal events it's not problem of the patch, similar patch as D15612 should be applied for that view.

In D14583#382850, @anthonyfieroni wrote:

If someone steal events it's not problem of the patch, similar patch as D15612 should be applied for that view.

Could you add that change to this patch too so that the issues folks have mentioned get resolved and we can finally land this?