I've added "skip news" because the relevant news item is in#112898

Please test with#112994. It shows different dialog windows (with and without the "Cancel" button) depending on the way the application is closed, so it can help you to understand what action is triggered.

I think that::tk::mac::Quit exists because there is a way to close the application on macOS other than via menu, keyboard shortcut or closing all windows.

Please test with#112994. It shows different dialog windows (with and without the "Cancel" button) depending on the way the application is closed, so it can help you to understand what action is triggered.

I think that::tk::mac::Quit exists because there is a way to close the application on macOS other than via menu, keyboard shortcut or closing all windows.

osascript -e 'tell application "IDLE" to quit', that's the event used during logging out. The update I just pushed works for that as well.

But as I wrote above: the added if statement inconfig.py isn't correct and needed in its current form because I've not yet found out whymacosx.isAquaTk() causes the app to ignore the quit handler.

But as I wrote above: the added if statement inconfig.py isn't correct and needed in its current form because I've not yet found out whymacosx.isAquaTk() causes the app to ignore the quit handler.

I agree that the windowing system should be checked instead, so I would like to understand why::tk::mac::Quit is ignored. But I notice that ifmacosx.isAquaTk() is checked instead, then quitting triggers a use-after-free bug in Tk Aqua, one with the same cause and workaround ashttps://core.tcl-lang.org/tk/info/1ab7a5f299

==76471==ERROR: AddressSanitizer: heap-use-after-free on address 0x61a0000a5ca8 at pc 0x00010bd99942 bp 0x7ff7bd8860f0 sp 0x7ff7bd8860e8READ of size 8 at 0x61a0000a5ca8 thread T0    #0 0x10bd99941 in Tcl_FindCommand tclNamesp.c:2553    #1 0x108e3ac6b in ReallyKillMe tkMacOSXHLEvents.c:594    #2 0x10bdc097b in Tcl_ServiceEvent tclNotify.c:670    #3 0x10bdc355b in Tcl_DoOneEvent tclNotify.c:967    #4 0x106d13c6c in _tkinter_tkapp_mainloop _tkinter.c.h:534    #5 0x1028f93b4 in method_vectorcall_FASTCALL descrobject.c:393    #6 0x1028b929e in PyObject_Vectorcall call.c:327    #7 0x102e953f3 in _PyEval_EvalFrameDefault generated_cases.c.h:815    #8 0x102e2c2a8 in PyEval_EvalCode ceval.c:592    #9 0x102e1c2c8 in builtin_exec bltinmodule.c.h:540    #10 0x102a7e4db in cfunction_vectorcall_FASTCALL_KEYWORDS methodobject.c:441    #11 0x1028b929e in PyObject_Vectorcall call.c:327    #12 0x102e953f3 in _PyEval_EvalFrameDefault generated_cases.c.h:815    #13 0x1028b904b in _PyVectorcall_Call call.c:273    #14 0x1031b53e2 in pymain_run_module main.c:297    #15 0x1031b29ce in Py_RunMain main.c:707    #16 0x1031b4a6a in pymain_main main.c:737    #17 0x1031b4ea8 in Py_BytesMain main.c:761    #18 0x113cdd52d in start+0x1cd (dyld:x86_64+0x552d)
0x61a0000a5ca8 is located 40 bytes inside of 1304-byte region [0x61a0000a5c80,0x61a0000a6198)freed by thread T0 here:    #0 0x10440ccf9 in free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x53cf9)    #1 0x10b0cfd1c in TclpFree tclAlloc.c:722    #2 0x10b1201bd in DeleteInterpProc tclBasic.c:1737    #3 0x10be6faae in Tcl_EventuallyFree tclPreserve.c:296    #4 0x10b11979c in Tcl_DeleteInterp tclBasic.c:1412    #5 0x106d08a60 in Tkapp_Dealloc _tkinter.c:2820    #6 0x102a3cf58 in _PyObject_FreeInstanceAttributes dictobject.c:5731    #7 0x102b5f8e3 in subtype_dealloc typeobject.c:2051    #8 0x102ffbf1c in _PyFrame_ClearExceptCode frame.c:140    #9 0x102eeb0b5 in clear_thread_frame ceval.c:1635    #10 0x102e4aa6e in _PyEval_EvalFrameDefault generated_cases.c.h:4853    #11 0x102e2c2a8 in PyEval_EvalCode ceval.c:592    #12 0x102e1c2c8 in builtin_exec bltinmodule.c.h:540    #13 0x102a7e4db in cfunction_vectorcall_FASTCALL_KEYWORDS methodobject.c:441    #14 0x1028b904b in _PyVectorcall_Call call.c:273    #15 0x102e5345f in _PyEval_EvalFrameDefault generated_cases.c.h:1250    #16 0x1028bd2b4 in object_vacall call.c:819    #17 0x1028bcc4d in PyObject_CallMethodObjArgs call.c:880    #18 0x10304aa7d in PyImport_ImportModuleLevelObject import.c:2837    #19 0x102e1802b in builtin___import__ bltinmodule.c.h:107    #20 0x102a7e4db in cfunction_vectorcall_FASTCALL_KEYWORDS methodobject.c:441    #21 0x1028b904b in _PyVectorcall_Call call.c:273    #22 0x102e5345f in _PyEval_EvalFrameDefault generated_cases.c.h:1250    #23 0x1028bd2b4 in object_vacall call.c:819    #24 0x1028bcc4d in PyObject_CallMethodObjArgs call.c:880    #25 0x10304ad49 in PyImport_ImportModuleLevelObject import.c:2905    #26 0x102e5439f in _PyEval_EvalFrameDefault generated_cases.c.h:2882    #27 0x102e16aa8 in builtin___build_class__ bltinmodule.c:205    #28 0x102a7e4db in cfunction_vectorcall_FASTCALL_KEYWORDS methodobject.c:441    #29 0x1028b929e in PyObject_Vectorcall call.c:327
previously allocated by thread T0 here:    #0 0x10440cbb0 in malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x53bb0)    #1 0x10b0cfcec in TclpAlloc tclAlloc.c:699    #2 0x10b1b199a in Tcl_Alloc tclCkalloc.c:1059    #3 0x10b10450a in Tcl_CreateInterp tclBasic.c:565    #4 0x106d06381 in Tkapp_New _tkinter.c:567    #5 0x106d041ba in _tkinter_create _tkinter.c.h:793    #6 0x102a7e3b6 in cfunction_vectorcall_FASTCALL methodobject.c:425    #7 0x1028b929e in PyObject_Vectorcall call.c:327    #8 0x102e953f3 in _PyEval_EvalFrameDefault generated_cases.c.h:815    #9 0x1028b61cf in _PyObject_VectorcallDictTstate call.c:135    #10 0x1028ba219 in _PyObject_Call_Prepend call.c:504    #11 0x102b67cd1 in slot_tp_init typeobject.c:9063    #12 0x102b4ebc8 in type_call typeobject.c:1687    #13 0x1028b662a in _PyObject_MakeTpCall call.c:242    #14 0x102e953f3 in _PyEval_EvalFrameDefault generated_cases.c.h:815    #15 0x102e2c2a8 in PyEval_EvalCode ceval.c:592    #16 0x102e1c2c8 in builtin_exec bltinmodule.c.h:540    #17 0x102a7e4db in cfunction_vectorcall_FASTCALL_KEYWORDS methodobject.c:441    #18 0x1028b904b in _PyVectorcall_Call call.c:273    #19 0x102e5345f in _PyEval_EvalFrameDefault generated_cases.c.h:1250    #20 0x1028bd2b4 in object_vacall call.c:819    #21 0x1028bcc4d in PyObject_CallMethodObjArgs call.c:880    #22 0x10304aa7d in PyImport_ImportModuleLevelObject import.c:2837    #23 0x102e1802b in builtin___import__ bltinmodule.c.h:107    #24 0x102a7e4db in cfunction_vectorcall_FASTCALL_KEYWORDS methodobject.c:441    #25 0x1028b904b in _PyVectorcall_Call call.c:273    #26 0x102e5345f in _PyEval_EvalFrameDefault generated_cases.c.h:1250    #27 0x1028bd2b4 in object_vacall call.c:819    #28 0x1028bcc4d in PyObject_CallMethodObjArgs call.c:880    #29 0x10304ad49 in PyImport_ImportModuleLevelObject import.c:2905

In Tk Aqua, the first Tk interpreter (i.e. the first Tcl interpreter to load Tk) is the one that will receive Apple high-level events, such as quitting. (The use-after-free bug I mentioned has to do with how Tk Aqua keeps a reference to the first Tk interpreter.) This means that defining::tk::mac::Quit is only effective when done from the first created Tk interpreter.

macosx.isAquaTk() relies on creating a newtkinter.Tk() instance, i.e. a separate Tk interpreter. Callingmacosx.isAquaTk() fromGetCoreKeys() ends up creating the first Tk interpreter, and so the::tk::mac::Quit definition inoverrideRootMenu() gets ignored.

If specifying bindings in config-keys.def is mandatory as I described in#112898 (comment) then a hacky workaround would be to specify a key which is nonexistent in practice but still recognized by Tk, such as F35:

--- a/Lib/idlelib/config-keys.def+++ b/Lib/idlelib/config-keys.def@@ -264,7 +264,7 @@ redo = <Shift-Command-Key-Z> close-window = <Command-Key-w> restart-shell = <Control-Key-F6> save-window-as-file = <Shift-Command-Key-S>-close-all-windows = <Command-Key-q>+close-all-windows = <Key-F35> view-restart = <Key-F6> tabify-region = <Control-Key-5> find-again = <Command-Key-g> <Key-F3>

Then there would be no need to modify idlelib/config.py.

But as I wrote above: the added if statement inconfig.py isn't correct and needed in its current form because I've not yet found out whymacosx.isAquaTk() causes the app to ignore the quit handler.

I agree that the windowing system should be checked instead, so I would like to understand why::tk::mac::Quit is ignored.

I haven't tried to debug this yet, other than noticing that IDLE quits without asking about unsafed data if I test usingmacosx.isAquaTk().

But I notice that ifmacosx.isAquaTk() is checked instead, then quitting triggers a use-after-free bug in Tk Aqua, one with the same cause and workaround ashttps://core.tcl-lang.org/tk/info/1ab7a5f299

ASan report:

:-(

If specifying bindings in config-keys.def is mandatory as I described in#112898 (comment) then a hacky workaround would be to specify a key which is nonexistent in practice but still recognized by Tk, such as F35:

--- a/Lib/idlelib/config-keys.def+++ b/Lib/idlelib/config-keys.def@@ -264,7 +264,7 @@ redo = <Shift-Command-Key-Z> close-window = <Command-Key-w> restart-shell = <Control-Key-F6> save-window-as-file = <Shift-Command-Key-S>-close-all-windows = <Command-Key-q>+close-all-windows = <Key-F35> view-restart = <Key-F6> tabify-region = <Control-Key-5> find-again = <Command-Key-g> <Key-F3>

Then there would be no need to modify idlelib/config.py.

That sadly doesn't work, IDLE just quits when quitting with unsaved files (both using the menu and Cmd-Q shortcut).

This is definitely related tomacosx.py:_init_tk_type(), I get the same behaviour when calling that function in the body ofmacosx.py (e.g. eagerly initialising the variable).

And that in turn is to be triggered by creating atkinter.Tk object in_init_tk_type. If I create a'::tk::mac::Quit command using that root the function registered there will get called and not the callable registered inoverrideRootMenu.

That's it for tonight.

The current PR works for me (TM), but contains code in macosx.py that I don't particularly like and still removes the<<close-all-windows>> binding in config.py (on AquaTk).

My latest commits to the PR clean up the code by tweaking the code a little to avoid having to create a temporarytkinter.Tk instance inmacosx._init_tk_type. This required some reordering inpyshell.main and required adding getters and setters foridlelib.mainmenu.default_keydefs, both to avoid needing to callmacosx._init_tk_type before IDLE's Tk root is created.

The code inmacosx._init_tk_type that accessesmacosx._idle_root is intentionally fragile, IDLE will fail to start up whenmacosx.isAquaTk is called before callingmacosx.setupApp (on macOS).

The current code looks mergeable to me now.

@terryjreedy, could you please review this PR? This fixes a real issue for macOS users (work may be lost when quitting IDLE, for example during system shutdown).

[Started before the last commit]
After reading#112898 (comment) and the following comment late last night, USA east coast time, I planned to write today a (separate) PR to change macosx._init_tk_type and pyshell.main to use the initial IDLE root when IDLE is running. But without deleting the temporary root when testing. It and the complexity of having the isXyz functions call _init_tk_type is only needed for testing, when the idlelib code called during the test happens to call one of the isXyz functions. Such calls may not be easy to predict. Hence the multiple test failures when the temp root was unconditionally deleted.

To fix the tests, there are multiple options.

1. Restore the temporary root when testing and delete if created. Something like

        if testing:            ...                requires('gui')                temroot = ...                ws = temroot.tk.call...        else:            ws = _idle_root...        ...        if testing and 'temroot' in locals: temroot.destroy()  # or try: temroot.destroy\nexcept NameError: pass2. (Long term easiest if possible.)  Always set _tk_type to "cocoa" when testing on Darwin.  I suggested in the existing comment, added in my last revision, that this might be possible (at least as long as we do not add macOS specific tests).  To test, run test_idle locally on non-cocoa machines after omitting the 'requires' test and see if anything fails.3. Add a call to _init_tk_type after the first root creation in the test files that have a failure.  Fragile if test-cases are reordered or the call is forgotten or not added when needed for new tests.   Though CI will now catch such.4. Change tests to create one root per test file, when needed anywhere, and call _init_tk_type.  Then gate gui-requiring test cases with `requires('gui')`.

[Started before the last commit] After reading#112898 (comment) and the following comment late last night, USA east coast time, I planned to write today a (separate) PR to change macosx._init_tk_type and pyshell.main to use the initial IDLE root when IDLE is running. But without deleting the temporary root when testing. It and the complexity of having the isXyz functions call _init_tk_type is only needed for testing, when the idlelib code called during the test happens to call one of the isXyz functions. Such calls may not be easy to predict. Hence the multiple test failures when the temp root was unconditionally deleted.

To fix the tests, there are multiple options.

1. Restore the temporary root when testing and delete if created. Something like

iftesting:            ...requires('gui')temroot= ...ws=temroot.tk.call...else:ws=_idle_root...        ...iftestingand'temroot'inlocals:temroot.destroy()# or try: temroot.destroy\nexcept NameError: pass

TBH I'm not a fan of theif testing block in the first place, therequires('gui') marker should be in tests that end up calling_init_tk type.

2. (Long term easiest if possible.) Always set _tk_type to "cocoa" when testing on Darwin. I suggested in the existing comment, added in my last revision, that this might be possible (at least as long as we do not add macOS specific tests). To test, run test_idle locally on non-cocoa machines after omitting the 'requires' test and see if anything fails.

This change would be fine with me, I'd be surprised if there's anyone left that would want to run (let alone test) the X11 based Tk backend on macOS (let alone the Carbon backend, that might not even exist in Tk 8.6)

That would IMHO require adding a test that asserts that the GUI type is "cocoa" on macOS to make it abundantly clear that this is the only supported setup.

3. Add a call to _init_tk_type after the first root creation in the test files that have a failure. Fragile if test-cases are reordered or the call is forgotten or not added when needed for new tests. Though CI will now catch such.

Currently all tests pass on macOS with this PR (alternate framework name to not affect a python.org install):

% /Library/Frameworks/DebugPython.framework/Versions/3.13/bin/python3 -m test -u gui test_tkinter                                                                                                                                                                   Using random seed: 9872695750:00:00 load avg: 2.39 Run 1 test sequentially0:00:00 load avg: 2.39 [1/1] test_tkinter== Tests result: SUCCESS ==1 test OK.Total duration: 10.7 secTotal tests: run=754 skipped=3Total test files: run=1/1Result: SUCCESS

You'll quickly notice if a test reorg requires changes because_init_tk_type will now raise an exception when called at the wrong time.

4. Change tests to create one root per test file, when needed anywhere, and call _init_tk_type. Then gate gui-requiring test cases withrequires('gui').

As@chrstphrchvz noted we should try to avoid more than one root, and that IMHO should include tests. We tend to run into edge cases in Tk when using multiple root's because Tcl users generally don't do this and also some functionality only works in the first root.

@terryjreedy, I've updated the tests as well they now pass on all platforms. I chose to mockmacosx._tk_type to avoid adding a dependency GUI testing just because some code callsmacosx.isAquaTk().

The tests assume that the GUI variant on macOS will be "cocoa", which is IMHO a safe assumption to make. The Carbon back-end to Tk is AFAIK long gone and X11 will be tested on other platforms.

Sorry about pushing, but what's needed to bring this PR over the finish line?

@terryjreedy, I intent to merge this tomorrow afternoon (CET timezone).

When you're done making the requested changes, leave the comment:I have made the requested changes; please review again.

Manual checks on Windows of saving, saving as, and closing with unsaved changes seems to behave the same without and with this patch.

I have made the requested changes; please review again.

Sorry about the slow response, I didn't have time to work on Python last weekend after all.

Ronald, I am leaving this for you to commit as I cannot test the macOS-specific changes except that they do not affect behavior on Windows. And that the cosmetic changes I requested are correct ;-).