Prime out essential interface attributes before impl initialization

[proneon267]

Running the following script:

import toga

class HelloWorld(toga.App):
    def startup(self):
        self.main_window = toga.MainWindow(title=self.formal_name)
        self.main_window.content = toga.ScrollContainer(content=toga.Box())
        self.main_window.show()

def main():
    return HelloWorld()

The following error occurs:

Traceback (most recent call last):
  File "/home/proneon267/venv/lib/python3.12/site-packages/toga_gtk/widgets/scrollcontainer.py", line 29, in gtk_on_changed
    self.interface.on_scroll()
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/proneon267/venv/lib/python3.12/site-packages/toga/widgets/scrollcontainer.py", line 148, in on_scroll
    return self._on_scroll
           ^^^^^^^^^^^^^^^
AttributeError: 'ScrollContainer' object has no attribute '_on_scroll'. Did you mean: 'on_scroll'?
Traceback (most recent call last):
  File "/home/proneon267/venv/lib/python3.12/site-packages/toga_gtk/widgets/scrollcontainer.py", line 29, in gtk_on_changed
    self.interface.on_scroll()
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/proneon267/venv/lib/python3.12/site-packages/toga/widgets/scrollcontainer.py", line 148, in on_scroll
    return self._on_scroll
           ^^^^^^^^^^^^^^^
AttributeError: 'ScrollContainer' object has no attribute '_on_scroll'. Did you mean: 'on_scroll'?

Context

It seems like the steps for initialization of widgets has recently been changed. For example, on testing by debug logging, the reported initialization steps of ScrollContainer() is:

|------Core: ScrollContainer: __init__(): Start
|    |------Core: Widget: __init__(): Start
|    |    |------Core: ScrollContainer: _create(): Start
|    |    |    |------Impl: Widget: __init__(): Start
|    |    |    |    |------Impl: ScrollContainer: create(): Start
|    |    |    |    |------Impl: ScrollContainer: create(): Complete
|    |    |    |------Impl: Widget: __init__(): Complete
|    |    |------Core: ScrollContainer: _create(): Complete
|    |------Core: Widget: __init__(): Complete
|------Core: ScrollContainer: __init__(): Complete

The error occurs in:

|------Core: ScrollContainer: __init__(): Start
|  |------Core: Widget: __init__(): Start
|  |  |------Core: ScrollContainer: _create(): Start
|  |  |  |------Impl: Widget: __init__(): Start
|  |  |  |  |------Impl: ScrollContainer: create(): Start
|  |  |  |  |  Traceback (most recent call last):
|  |  |  |  |    File "/home/proneon267/toga/gtk/src/toga_gtk/widgets/scrollcontainer.py", line 29, in gtk_on_changed
|  |  |  |  |      self.interface.on_scroll()
|  |  |  |  |      ^^^^^^^^^^^^^^^^^^^^^^^^
|  |  |  |  |    File "/home/proneon267/toga/core/src/toga/widgets/scrollcontainer.py", line 148, in on_scroll
|  |  |  |  |      return self._on_scroll
|  |  |  |  |            ^^^^^^^^^^^^^^^
|  |  |  |  |  AttributeError: 'ScrollContainer' object has no attribute '_on_scroll'. Did you mean: 'on_scroll'?
|  |  |  |  |  Traceback (most recent call last):
|  |  |  |  |    File "/home/proneon267/toga/gtk/src/toga_gtk/widgets/scrollcontainer.py", line 29, in gtk_on_changed
|  |  |  |  |      self.interface.on_scroll()
|  |  |  |  |      ^^^^^^^^^^^^^^^^^^^^^^^^
|  |  |  |  |    File "/home/proneon267/toga/core/src/toga/widgets/scrollcontainer.py", line 148, in on_scroll
|  |  |  |  |      return self._on_scroll
|  |  |  |  |            ^^^^^^^^^^^^^^^
|  |  |  |  |  AttributeError: 'ScrollContainer' object has no attribute '_on_scroll'. Did you mean: 'on_scroll'?
|  |  |  |  |------Impl: ScrollContainer: create(): Complete
|  |  |  |------Impl: Widget: __init__(): Complete
|  |  |------Core: ScrollContainer: _create(): Complete
|  |------Core: Widget: __init__(): Complete
|------Core: ScrollContainer: __init__(): Complete

The gtk event changed gets triggered by the backend early on, and since the ScrollContainer interface(core) initializer calls the Impl initializer while not being completely initialized itself(interface), the gtk callback event throws an error. To reduce code churn, I have gone with the simplest fix.

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[freakboy3742]

Thanks for the report; Your first attempt got the right general idea for the fix (in the implementation layer); the subsequent fix on the GTK backend isn't correct.

The old implementation set on_scroll = None for exactly the same reason as you've identified - because the widget fired the event handler when the widget is created, so a handler needs to be primed first. However, in #2942, the order of widget instantiation was slightly modified, and as a result, the priming needs to occur before super().__init__() is invoked.

The assignment of _content was historically for the same reason.

From an inspection of the code, I think there is a similar potential bug in NumberInput, OptionContainer, Selection, and maybe Canvas and SplitContainer. These might not be triggered in practice, but the potential is there.

[proneon267]

I agree, the fix on the interface is correct. But, somehow the dummy backend is triggering the on_scroll event on initialization. So, I guess there are some changes to be made on the dummy backend too.

[freakboy3742]

I agree, the fix on the interface is correct. But, somehow the dummy backend is triggering the on_scroll event on initialization. So, I guess there are some changes to be made on the dummy backend too.

No, that's my entire point. The on_scroll handler needs to be primed before the impl is created. Previously, it was, because the call to create the impl was explicit, and it occurred after on_scroll was set to None. Now, the impl is created as part of the call to super().__init__, and on_scroll isn't instantiated until after the widget is created.

I'm more intrigued as to why the current code passes both the GTK testbed and the core tests.

[proneon267]

No, that's my entire point. The on_scroll handler needs to be primed before the impl is created.

Yes, I meant that we would need to prime the attributes that might be used during the impl initialization, before calling super().__init__

But the current failed core test indicate that on the dummy backend, the on_scroll event is being triggered twice during the initialization. So, something is not correct either on the core or dummy.
Correctly priming the attributes works correctly on both core tests and on native gtk.

I'm more intrigued as to why the current code passes both the GTK testbed and the core tests.

I am not sure, but the AssertionError might be occuring at a time which the pytest might consider outside the purview of the test, and so discards it. Even a simple test, doesn't trigger the error, when pytest is run with -s:

async def test_scrollcontainer_error():
    _ = toga.ScrollContainer(content=toga.Box())

But just running the example triggers the AssertionError.

[proneon267]

Ok, so correctly priming the attributes fixes the bug. But, I am still not sure why pytest is not reporting the AssertionError on testbed.

[freakboy3742]

The ordering for canvas is slightly off, which I've corrected; otherwise, this looks great. Thanks for the fix!

[freakboy3742]

This placement isn't correct. The live event handlers shouldn't be in place before the widget is created. The comparison should be with the old implementation.

[proneon267]

Thanks. Also, did you find anything regarding pytest not reporting the error?

[freakboy3742]

I didn't; I can only assume that because the test is running with no delays, the signal propagation order is slightly different, and as a result, the error doesn't surface.

[HalfWhitt]

Thank you for catching this and cleaning up after me! Definitely perplexing about pytest not being able to catch it. Even though these are now fixed, it is a little worrying having a class of potential errors that isn't (as far as we know so far) testable.