Revert “gui: provide wallet controller context to wallet actions” #770

1. 
   hebasto commented at 11:19 am on October 23, 2023: member

   The commit 7066e8996d0ac090535cc97cdcb54a219986460f from #765 breaks “Open Wallet”, “Close Wallet” and “Close All Wallets” items in the File menu (at least on Ubuntu 23.10 + Wayland).

   Reverting it to avoid this regression in the 26.0 release.

2. Revert "gui: provide wallet controller context to wallet actions"

   This reverts commit 7066e8996d0ac090535cc97cdcb54a219986460f.

   f09bfab4af
3. hebasto added this to the milestone 26.0 on Oct 23, 2023
4. 
   DrahtBot commented at 11:20 am on October 23, 2023: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	furszy, jarolrod

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

5. 
   hebasto commented at 11:20 am on October 23, 2023: member

   cc @furszy @jarolrod @Sjors
6. MarnixCroes approved
7. 
   MarnixCroes commented at 12:01 pm on October 23, 2023: contributor

   tack f09bfab4aff04a9cc1ea157b94bfdae19f3465b1
8. 
   furszy commented at 12:39 pm on October 23, 2023: member

   Upps.. it is because m_wallet_controller is null when the events are connected (at widget creation time). We need to move the connection to setWalletController(), otherwise there is also a possible crash that could arise when any of these events is triggered prior to the wallet controller is set.
9. 
   furszy commented at 12:40 pm on October 23, 2023: member

   ACK f09bfab4 for including it in v26.
10. 
    fanquake commented at 12:46 pm on October 23, 2023: member

    Upps.. it is because m_wallet_controller is null when the events are connected (at widget creation time). We need to move the connection to setWalletController(), otherwise there is also a possible crash that could arise when any of these events is triggered prior to the wallet controller is set.

    So this isn’t actually platform specific, it was just always broken?

11. 
    furszy commented at 12:52 pm on October 23, 2023: member

    Upps.. it is because m_wallet_controller is null when the events are connected (at widget creation time). We need to move the connection to setWalletController(), otherwise there is also a possible crash that could arise when any of these events is triggered prior to the wallet controller is set.

    So this isn’t actually platform specific, it was just always broken?

    Since #765 merge, last week. Yes. It tell us how much we need to improve the GUI test coverage (and its code structure..). We fixed an issue and broke something else.

12. furszy approved
13. jarolrod approved
14. 
    jarolrod commented at 1:48 pm on October 23, 2023: member

    ACK f09bfab4aff04a9cc1ea157b94bfdae19f3465b1

    The specified menu options don’t lead to crash on macOS, but are unusable. This was an oversight in review of #765. The specified menu actions work again with this branch.

15. hebasto merged this on Oct 23, 2023
16. hebasto closed this on Oct 23, 2023

    ---

17. hebasto deleted the branch on Oct 23, 2023
18. Frank-GER referenced this in commit 305b8ba609 on Nov 28, 2023
19. bitcoin-core locked this on Oct 22, 2024

Contributors
hebasto DrahtBot MarnixCroes furszy fanquake jarolrod

Milestone
26.0