Repository: qutebrowser/qutebrowser
Pull request: fix mac scrolling through tabbar (issue #6148)
<!-- Thanks for submitting a pull request! Please pick a descriptive title (not just "issue 12345"). If there is an open issue associated to your PR, please add a line like "Closes #12345" somewhere in the PR description (outside of this comment) -->

Closes #6148 

I have implemented what I think is a fix for issue #6148.  Let me know if I have done this properly, or if it needs changes.  Thanks!

Problem statement:
macos: can't scroll through tabbar
**Version info**:
<!-- Please copy the first block from :version, not just the qutebrowser version -->
qutebrowser v2.0.2
Git commit: f11cdd0bf on HEAD (2021-02-04 10:19:33 +0100)
Backend: QtWebEngine (Chromium 83.0.4103.122)
Qt: 5.15.2

**Does the bug happen if you start with `--temp-basedir`?**:
yes

**Description**
running the scrollwheel with the mouse hovered over the tabbar on macos doesn't cycle through tabs

**How to reproduce**
NA/description

Hints from issue discussion:
I think Qt does some special stuff to use macOS' tab implementation for its `QTabBar`, which might be causing this. I'll try to take a closer look sometime soon, haven't gotten around to it yet.
I can reproduce - it looks like this is deliberately ignored in Qt:

- [[QTBUG-8428] wheel event should not change active tab - Qt Bug Tracker](https://bugreports.qt.io/browse/QTBUG-8428)
- [QTabBar: Disable wheel events on Mac (Ifab663b4) · Gerrit Code Review](https://codereview.qt-project.org/c/qt/qtbase/+/109523)

Here's the Qt 5.15.2 doing so: https://github.com/qt/qtbase/blob/v5.15.2/src/widgets/widgets/qtabbar.cpp#L2325-L2352
It looks like Qt 6 made it configurable: https://github.com/qt/qtbase/blob/v6.1.0-alpha1/src/widgets/widgets/qtabbar.cpp#L2363-L2386 ([control scrolling of QTabBar using StyleHint (I99eeb5a1) · Gerrit Code Review](https://codereview.qt-project.org/c/qt/qtbase/+/327746)).

We already override `wheelEvent` for the `tabs.mousewheel_switching` option, so we might as well just reimplement those couple of lines of logic for macOS instead of relying on the Qt implementation...

Commit message: fix mac scrolling through tabbar (issue #6148)

Patch to review:
diff --git a/qutebrowser/mainwindow/tabwidget.py b/qutebrowser/mainwindow/tabwidget.py
index 4041de2c915..0d8c39f9a8b 100644
--- a/qutebrowser/mainwindow/tabwidget.py
+++ b/qutebrowser/mainwindow/tabwidget.py
@@ -705,7 +705,17 @@ def wheelEvent(self, e):
             e: The QWheelEvent
         """
         if config.val.tabs.mousewheel_switching:
-            super().wheelEvent(e)
+            if utils.is_mac:
+                index = self.currentIndex()
+                if index != -1:
+                    if e.angleDelta().y() > 0:
+                        index += -1
+                    else:
+                        index += 1
+                    if index >= 0 and index < self.count():
+                        self.setCurrentIndex(index)
+            else:
+                super().wheelEvent(e)
         else:
             tabbed_browser = objreg.get('tabbed-browser', scope='window',
                                         window=self._win_id)


Code review comments:
diff_hunk: @@ -705,7 +705,17 @@ def wheelEvent(self, e):
             e: The QWheelEvent
         """
         if config.val.tabs.mousewheel_switching:
-            super().wheelEvent(e)
+            if utils.is_mac:
+                index = self.currentIndex()
+                if index != -1:
+                    if e.angleDelta().y() > 0:
+                        index += -1
+                    else:
+                        index += 1
path: qutebrowser/mainwindow/tabwidget.py
text: @user1:
This could be

```suggestion
                    offset = -1 if e.angleDelta().y() > 0 else 1
                    index += offset
```

to make it (IMHO) a bit easier to read. I'm okay with both versions though. If you decide to keep yours, use `-= 1` instead of `+= -1` though.

diff_hunk: @@ -705,7 +705,17 @@ def wheelEvent(self, e):
             e: The QWheelEvent
         """
         if config.val.tabs.mousewheel_switching:
-            super().wheelEvent(e)
+            if utils.is_mac:
+                index = self.currentIndex()
+                if index != -1:
path: qutebrowser/mainwindow/tabwidget.py
text: @user1:
If you do:

```suggestion
                if index == -1:
                    return
```

here instead, you can dedent the rest by one, making this a bit simpler to read.

diff_hunk: @@ -705,7 +705,17 @@ def wheelEvent(self, e):
             e: The QWheelEvent
         """
         if config.val.tabs.mousewheel_switching:
-            super().wheelEvent(e)
+            if utils.is_mac:
+                index = self.currentIndex()
+                if index != -1:
+                    if e.angleDelta().y() > 0:
+                        index += -1
+                    else:
+                        index += 1
+                    if index >= 0 and index < self.count():
path: qutebrowser/mainwindow/tabwidget.py
text: @user1:
You can chain comparisons in Python to check if something is in a certain range:

```suggestion
                    if 0 <= index < self.count():
```

diff_hunk: @@ -705,7 +705,17 @@ def wheelEvent(self, e):
             e: The QWheelEvent
         """
         if config.val.tabs.mousewheel_switching:
-            super().wheelEvent(e)
+            if utils.is_mac:
+                index = self.currentIndex()
+                if index != -1:
+                    if e.angleDelta().y() > 0:
path: qutebrowser/mainwindow/tabwidget.py
text: @user1:
Qt's implementation checks the x and y deltas to make it possible to scroll with a wheel supporting horizontal scrolling as well:

https://github.com/qt/qtbase/blob/v5.15.2/src/widgets/widgets/qtabbar.cpp#L2325-L2352

We should probably do something similar too, to make both horizontal or vertical scrolling work?

@author:
I used the same logic that you linked to, so scrolling in either direction should work.