Refactor disable/enable logic for statusbar widgets

[The-Compiler]

As evident in #7389, handling of enabling/disabling statusbar widgets is rather error-prone at the moment: bar.py needs to have special handling for some widgets which handle their visibility themselves (progress.py and backforward.py).

We should instead unify this somehow in the API a statusbar item exposes. One possibility would be to give them a .enable() and .disable() method, which then either just call self.hide() and self.show(), or instead set self._enabled accordingly.

First draft:

diff --git i/qutebrowser/mainwindow/statusbar/backforward.py w/qutebrowser/mainwindow/statusbar/backforward.py
index 76614a14f..d3cdf987f 100644
--- i/qutebrowser/mainwindow/statusbar/backforward.py
+++ w/qutebrowser/mainwindow/statusbar/backforward.py
@@ -28,7 +28,14 @@ class Backforward(textbase.TextBase):
 
     def __init__(self, parent=None):
         super().__init__(parent)
-        self.enabled = False
+        self._enabled = False
+
+    def enable(self):
+        self._enabled = True
+
+    def disable(self):
+        self._enabled = False
+        self.hide()
 
     def on_tab_cur_url_changed(self, tabs):
         """Called on URL changes."""
@@ -49,4 +56,4 @@ def on_tab_changed(self, tab):
         if text:
             text = '[' + text + ']'
         self.setText(text)
-        self.setVisible(bool(text) and self.enabled)
+        self.setVisible(bool(text) and self._enabled)
diff --git i/qutebrowser/mainwindow/statusbar/bar.py w/qutebrowser/mainwindow/statusbar/bar.py
index 7e5a8ff2c..451a1665b 100644
--- i/qutebrowser/mainwindow/statusbar/bar.py
+++ w/qutebrowser/mainwindow/statusbar/bar.py
@@ -262,13 +262,8 @@ def _draw_widgets(self):
             if segment == 'scroll_raw':
                 widget.set_raw()
             elif segment in ('history', 'progress'):
-                widget.enabled = True
                 if tab:
                     widget.on_tab_changed(tab)
-
-                # Do not call .show() for these widgets. They are not always shown, and
-                # dynamically show/hide themselves in their on_tab_changed() methods.
-                continue
             elif segment.startswith('text:'):
                 widget.setText(segment.split(':', maxsplit=1)[1])
             elif segment.startswith('clock:') or segment == 'clock':
@@ -278,7 +273,7 @@ def _draw_widgets(self):
                 else:
                     widget.format = '%X'
 
-            widget.show()
+            widget.enable()
 
     def _clear_widgets(self):
         """Clear widgets before redrawing them."""
@@ -287,9 +282,7 @@ def _clear_widgets(self):
                        self.backforward, self.tabindex,
                        self.keystring, self.prog, self.clock, *self._text_widgets]:
             assert isinstance(widget, QWidget)
-            if widget in [self.prog, self.backforward]:
-                widget.enabled = False  # type: ignore[attr-defined]
-            widget.hide()
+            widget.disable()
             self._hbox.removeWidget(widget)
         self._text_widgets.clear()
 
diff --git i/qutebrowser/mainwindow/statusbar/progress.py w/qutebrowser/mainwindow/statusbar/progress.py
index 9d56cd03f..4a49f32f5 100644
--- i/qutebrowser/mainwindow/statusbar/progress.py
+++ w/qutebrowser/mainwindow/statusbar/progress.py
@@ -46,11 +46,18 @@ class Progress(QProgressBar):
     def __init__(self, parent=None):
         super().__init__(parent)
         stylesheet.set_register(self)
-        self.enabled = False
+        self._enabled = False
         self.setSizePolicy(QSizePolicy.Fixed, QSizePolicy.Fixed)
         self.setTextVisible(False)
         self.hide()
 
+    def enable(self):
+        self._enabled = True
+
+    def disable(self):
+        self._enabled = False
+        self.hide()
+
     def __repr__(self):
         return utils.get_repr(self, value=self.value())
 

but the main problem with that approach is that we have a lot of copy-paste-code.

Would be nice to have a common base class for statusbar widgets, but they also inherit from whatever widget they want to display... Possible solutions:

• Make widgets StatusbarItem subclasses, and have the widget as .widget instead of inheriting from it directly - somewhat similar to what we did for TabbedBrowser in e169e21 essentially. Seems like the cleanest option to me.
• Simply have two different mixins for enable/disable handling, and perhaps a typing.Protocol to statically check that they are there in bar.py somehow
• Use a miscwidgets.WrapperLayout like we do for browser tabs, so that statusbar items are widgets, but with our own API instead of the QWidget one.

cc @tomiesz @Nicholas42 in case you're interested!

[tomiesz]

I'm not available for ~2 weeks from now. If nobody gets this done in the meantime I'd gladly take a stab at it.

[toofar]

Couple more notes:

• @tomiesz already pointed on Fix for dissapearing Widgets #7390 that QWidget already has setEnabled() and isEnabled() methods, maybe we should be using them instead of having our own ones. I'm not sure if there are any complications around that stuff or not (the docs say that is for when the widget is disabled but maybe still visible (so greyed out), does that fit how we are using them? maybe)
• I'm not sure if it's too big a bit to take on under the scope of just this issue but eventually I would like the StatusBar widget to know nothing about any individual status bar item. I think how I would tackle that would be
  • make all the individual widgets inherit from a common base class
  • add the hardcoded widgets to a list
  • in each place the hardcoded widgets are accessed at the moment turn that into a loop over the list that calls generic methods of the new base class, the child widgets would override these generic methods and do the work that's being done in the StatusBar class now
  • add a decorator so the individual items can be dynamically registered instead of having a list of them maintained in the bar file
  • think about adding the new base class and decorator into the extension API
  • the txt and cmd items might continue to need special treatment, or maybe the relationship between them can be pushed down into the command widget

[4nd3r]

Maybe #4894 can be also included in statusbar refactoring? Or at least lay the groundwork.

[pylbrecht]

I am going to take a stab at this.

[pylbrecht]

I did some spiking over the past weeks and now have a somewhat clear implementation idea.

@toofar, as you predicted, writing an integration test for the StatusBar class is tricky. I will put together a draft PR of my implementation idea to collect some feedback before going all-in.

[toofar]

@pylbrecht that's great to hear! I'm looking forward to the sketch.

[pylbrecht]

Here you go: #8203

Feedback is very much appreciated!