Tk Source Code

In Tk 8.6.12, setting the `-type` option of a Tk menu, causes it to segfault:
menu .m -type {}

Using `configure` does not cause it to segfault:
menu .m
.m configure -type {}

On Tk 8.6.11, it appeared to be silently ignored (was this correct behaviour?).

Non-regression test menu-39.1 now merged into core-8-6-branch and trunk.

For the record, the discussion in this ticket prompted the idea exposed in TIP #613.

Thank you for this information regarding the context in which the issue was found, this is always interesting.

I have now committed a non-regression test (menu-39.1).

Besides, I have looked at all (remaining) occurrences of parameters of type TK_OPTION_STRING_TABLE having the TK_OPTION_NULL_OK flag set in Tk. There are exactly four such cases: the -tabstyle and -wrap options of the text widget, the -compound option of the ttk::button, and the -compound option of tabs of the ttk::notebook widget. I have tested each of them and have found that the {} case passed to the option is already correctly handled for each of them so there is nothing more to fix I think.

Similar to https://core.tcl-lang.org/tk/tktview/46c2f088, this came to light
because of https://bugs.python.org/issue45436. The CPython tkinter test suite has
not raised any other issues with empty strings being passed as an option value,
but I don't know how comprehensive the suite is.

OK, that's fine.

Before closing this ticket I believe we should check whether the same issue could hit from elsewhere in the code. This boils down to looking at where TK_OPTION_STRING_TABLE is used in the code and for which parameter, and check for presence of the TK_OPTION_NULL_OK flag in these parameters declarations.

Also a non-regression check test for the present ticket would not hurt.

I'll deal with the two points to above.

Thinking about it more, I like your (@fvogel) fix more (at least for 8.6). So, merged now. Thanks!

Alternative fix [d0b89a43de0b3da|here]. Since this fix changes behavior (although all test-cases pass...) I prefer your fix for 8.6. For 8.7 we could consider making it more consistent.

My guess is that with this commit it started failing: [bc258ad6eae49a08]

Explanation: The TK_OPTION_STRING_TABLE option never supported TK_OPTION_NULL_OK, so it broke when TK_OPTION_NULL_OK was implemented for TK_OPTION_STRING_TABLE.

Another possible fix would be to accept NULL in the code, it would mean that "" is no longer ambiguous but synonym with "normal". That would be a change in behaviour, but it would be consistent with ".m configure -type" accepting the empty string as being equivalent with "normal" as well.

Thank you for your report!

Commit [94687a026d] fixes this, but before merging it I would like to understand how come it worked in previous versions of Tk, whic I have not yet figured out yet.

I have checked that 8.6.9 for instance returns the correct answer:

ambiguous type "": must be normal, tearoff, or menubar