bpo-25684: ttk.OptionMenu radiobuttons aren't unique between instance… #2276
Conversation
|
@csabella, thanks for your PR! By analyzing the history of the files in this pull request, we identified @gpolo, @serhiy-storchaka and @vadmium to be potential reviewers. |
| @@ -276,7 +276,25 @@ def test_menu(self): | |||
| self.assertRaises(tkinter.TclError, optmenu['menu'].invoke, -1) | |||
| self.assertEqual(optmenu._variable.get(), items[0]) | |||
|
|
|||
| # check that radiobuttons are unique across instances (bpo25684) | |||
There was a problem hiding this comment.
Could you please move this test into separate method?
serhiy-storchaka
added
needs backport to 2.7
type-bug
|
Please add an entry in |
|
@serhiy-storchaka Thank you for the review. I've made the requested changes. It was the first time I've added a NEWS entry though, so hopefully I used blurb correctly. |
| items[2]) | ||
|
|
||
| optmenu.destroy() | ||
| del textvar2, optmenu_radiobutton_name, optmenu2_radiobutton_name |
There was a problem hiding this comment.
Not needed. del just unbinds the local names. This happens anyway when the function exits.
There was a problem hiding this comment.
Thanks. For some reason, even though they were local, I thought that the Tk variables had to be explicitly deleted for refleaks. But, it's probably just when it's in setUp.
| optmenu['menu'].invoke(1) | ||
| optmenu2['menu'].invoke(2) | ||
| optmenu_radiobutton_name = optmenu['menu'].entrycget(0, 'variable') | ||
| optmenu2_radiobutton_name = optmenu2['menu'].entrycget(0, 'variable') |
There was a problem hiding this comment.
I presume that this is actually the tk name of the StringVar that has the selected value.
If so, optmenu_stringvar_name would seem clearer. With the bug fixed, the name should be 'PY_VAR####' instead of the current ttk default '::selectedButton'.
| self.assertEqual(self.root.tk.globalgetvar(optmenu_radiobutton_name), | ||
| items[1]) | ||
| self.assertEqual(self.root.tk.globalgetvar(optmenu2_radiobutton_name), | ||
| items[2]) |
There was a problem hiding this comment.
I cannot find any doc for globalgetvar other than its use in the Variable subclass get methods. It appear to be a _tkinter function that gets the value of a Variable from its name.
There was a problem hiding this comment.
I had a lot of trouble with this when I was trying to create the test. I had even asked for help on core_workflow. But, after looking through the other tests in this suite, I saw that globalgetvar gave me what I wanted. I ran the test over the old code and it failed (which was good - failed on the old and passed on the new) and I also looked at the values in debug and this was exactly what I needed. I don't know if there was any other way to get that information.
|
Although I approved, I also suggested a change. The last commit satisfies Serhiy request. |
|
The release candidate for the final bugfix release of 3.5, 3.5.4, is out. Since this is not a security issue, I believe it should not be backported to 3.5 |
|
Terry, Yes, I can use cherry_picker to backport this once it's merged. Thanks! |
|
Thank you @csabella! When you will backport this PR to 3.6 and 2.7 don't forgot to add your name in |
|
GH-2959 is a backport of this pull request to the 3.6 branch. |
between instances of OptionMenu. (cherry picked from commit a568e52)
|
GH-2960 is a backport of this pull request to the 2.7 branch. |
…s of OptionMenu
Solution based on workaround by Bryan Oakley at https://stackoverflow.com/questions/33831289/ttk-optionmenu-displaying-check-mark-on-all-menus
to set the
variableattribute of the OptionMenu when the radiobuttons are added.https://bugs.python.org/issue25684