[Bug 2150178] Re: update-manager theoretically crashes due to Gtk.Window.run() in BuilderDialog

[Arnab Patra]

Hi,

I looked deeper into the BuilderDialog / run() / window_dialog flow, and
I think the situation is a bit more subtle than simple dead code
removal.

### 1. run() is still part of a live polymorphic interface

Even though BuilderDialog.run() is marked as deprecated/ignored, I found
that:

- Core/utils.py calls `d.run()`
- InstallBackendAptdaemon and other dialogs also call `dialog.run()`
- Subclasses like DistUpgradeDialog override `run(self, parent)`

So `run()` is still part of the active dialog lifecycle contract.

Because of this, removing `run()` entirely would likely break existing
dialog flows, even if BuilderDialog’s own implementation is currently a
no-op.

---

### 2. window_dialog is inconsistently assumed across subclasses

>From tracing the UI flow:

- `window_dialog` is only defined in `data/gtkbuilder/Dialog.ui`
- It is only reliably loaded through InternalDialog → BuilderDialog initialization path
- Some backend dialogs (e.g. InstallBackendAptdaemon) assume `window_dialog` exists, but in practice it is not guaranteed in all code paths

This is consistent with the crash:

  'InstallBackendAptdaemon' object has no attribute 'window_dialog'

So the issue seems to be that `window_dialog` is part of an implicit UI
contract that is no longer consistently enforced.

---

### 3. The core problem seems to be contract inconsistency, not dead
code

Right now we have a mismatch between layers:

- Core expects: `dialog.run()` always works
- UI layer: BuilderDialog.run() exists but is deprecated/ignored
- Backend: assumes UI attributes like `window_dialog` exist even when not initialized

So the real issue appears to be:
   broken or inconsistent dialog lifecycle contract across subclasses

---

### 4. Concern about removing run() and window_dialog directly

I agree both are legacy and confusing, but removing them outright seems
risky because:

- `run()` is still part of active call paths (`d.run()`)
- some subclasses override it with real logic
- backend dialogs still depend on dialog lifecycle flow
- window_dialog may still be expected in some GTK builder flows

So direct removal could introduce regressions in upgrade/install paths
that are not immediately visible.

---

### 5. Suggested safer direction

Instead of removal first, it may be safer to:

- First standardize dialog initialization (ensure consistent UI state creation)
- Explicitly define whether `window_dialog` is required or optional in base class
- Clarify `run()` contract (either abstract or properly implemented base behavior)
- Then deprecate/remove once usage paths are fully confirmed dead

---

### Summary

I think your suggestion about cleanup is valid, but my concern is mainly about sequencing:
we may need to stabilize/normalize the dialog contract first before removing these components, otherwise we risk breaking hidden runtime paths.

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to update-manager in Ubuntu.
https://bugs.launchpad.net/bugs/2150178

Title:
  update-manager theoretically crashes due to Gtk.Window.run() in
  BuilderDialog

Status in update-manager package in Ubuntu:
  Triaged

Bug description:
  In UpdateManager/Dialogs.py, BuilderDialog.run() calls
  self.window_dialog.run(), which causes a crash because
  Gtk.Window does not support the run() method (only Gtk.Dialog does).

  This leads to a runtime error when the dialog is shown.

  Proposed fix:
  Replace run() with show_all(), since update-manager already runs
  inside the GTK main loop.

  Patch attached.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/update-manager/+bug/2150178/+subscriptions