Skip to content

Commit

Permalink
bpo-42630: Improve error reporting in Tkinter for absent default root (
Browse files Browse the repository at this point in the history
…pythonGH-23781)

* Tkinter functions and constructors which need a default root window
  raise now RuntimeError with descriptive message instead of obscure
  AttributeError or NameError if it is not created yet or cannot
  be created automatically.

* Add tests for all functions which use default root window.

* Fix import in the pynche script.
  • Loading branch information
serhiy-storchaka authored Dec 19, 2020
1 parent 1e27b57 commit 3d569fd
Show file tree
Hide file tree
Showing 19 changed files with 315 additions and 87 deletions.
4 changes: 3 additions & 1 deletion Lib/idlelib/pyshell.py
Original file line number Diff line number Diff line change
Expand Up @@ -1061,8 +1061,10 @@ def begin(self):
(sys.version, sys.platform, self.COPYRIGHT, nosub))
self.text.focus_force()
self.showprompt()
# User code should use separate default Tk root window
import tkinter
tkinter._default_root = None # 03Jan04 KBK What's this?
tkinter._support_default_root = True
tkinter._default_root = None
return True

def stop_readline(self):
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_idle.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@
if __name__ == '__main__':
tk.NoDefaultRoot()
unittest.main(exit=False)
tk._support_default_root = 1
tk._support_default_root = True
tk._default_root = None
51 changes: 30 additions & 21 deletions Lib/tkinter/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ def __repr__(self):
)


_support_default_root = 1
_support_default_root = True
_default_root = None


Expand All @@ -280,13 +280,26 @@ def NoDefaultRoot():
Call this function to inhibit that the first instance of
Tk is used for windows without an explicit parent window.
"""
global _support_default_root
_support_default_root = 0
global _default_root
global _support_default_root, _default_root
_support_default_root = False
# Delete, so any use of _default_root will immediately raise an exception.
# Rebind before deletion, so repeated calls will not fail.
_default_root = None
del _default_root


def _get_default_root(what=None):
if not _support_default_root:
raise RuntimeError("No master specified and tkinter is "
"configured to not support default root")
if not _default_root:
if what:
raise RuntimeError(f"Too early to {what}: no default root window")
root = Tk()
assert _default_root is root
return _default_root


def _tkerror(err):
"""Internal function."""
pass
Expand Down Expand Up @@ -330,7 +343,7 @@ def __init__(self, master=None, value=None, name=None):
raise TypeError("name must be a string")
global _varnum
if not master:
master = _default_root
master = _get_default_root('create variable')
self._root = master._root()
self._tk = master.tk
if name:
Expand Down Expand Up @@ -591,7 +604,7 @@ def get(self):

def mainloop(n=0):
"""Run the main loop of Tcl."""
_default_root.tk.mainloop(n)
_get_default_root('run the main loop').tk.mainloop(n)


getint = int
Expand All @@ -600,9 +613,9 @@ def mainloop(n=0):


def getboolean(s):
"""Convert true and false to integer values 1 and 0."""
"""Convert Tcl object to True or False."""
try:
return _default_root.tk.getboolean(s)
return _get_default_root('use getboolean()').tk.getboolean(s)
except TclError:
raise ValueError("invalid literal for getboolean()")

Expand Down Expand Up @@ -2248,7 +2261,7 @@ def __init__(self, screenName=None, baseName=None, className='Tk',
is the name of the widget class."""
self.master = None
self.children = {}
self._tkloaded = 0
self._tkloaded = False
# to avoid recursions in the getattr code in case of failure, we
# ensure that self.tk is always _something_.
self.tk = None
Expand All @@ -2272,7 +2285,7 @@ def loadtk(self):
self._loadtk()

def _loadtk(self):
self._tkloaded = 1
self._tkloaded = True
global _default_root
# Version sanity checks
tk_version = self.tk.getvar('tk_version')
Expand Down Expand Up @@ -2521,12 +2534,8 @@ class BaseWidget(Misc):

def _setup(self, master, cnf):
"""Internal function. Sets up information about children."""
if _support_default_root:
global _default_root
if not master:
if not _default_root:
_default_root = Tk()
master = _default_root
if not master:
master = _get_default_root()
self.master = master
self.tk = master.tk
name = None
Expand Down Expand Up @@ -3990,9 +3999,7 @@ class Image:
def __init__(self, imgtype, name=None, cnf={}, master=None, **kw):
self.name = None
if not master:
master = _default_root
if not master:
raise RuntimeError('Too early to create image')
master = _get_default_root('create image')
self.tk = getattr(master, 'tk', master)
if not name:
Image._last_id += 1
Expand Down Expand Up @@ -4146,11 +4153,13 @@ def __init__(self, name=None, cnf={}, master=None, **kw):


def image_names():
return _default_root.tk.splitlist(_default_root.tk.call('image', 'names'))
tk = _get_default_root('use image_names()').tk
return tk.splitlist(tk.call('image', 'names'))


def image_types():
return _default_root.tk.splitlist(_default_root.tk.call('image', 'types'))
tk = _get_default_root('use image_types()').tk
return tk.splitlist(tk.call('image', 'types'))


class Spinbox(Widget, XView):
Expand Down
4 changes: 2 additions & 2 deletions Lib/tkinter/commondialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ class Dialog:
command = None

def __init__(self, master=None, **options):
if not master:
master = options.get('parent')
self.master = master
self.options = options
if not master and options.get('parent'):
self.master = options['parent']

def _fixoptions(self):
pass # hook
Expand Down
6 changes: 3 additions & 3 deletions Lib/tkinter/font.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def _mkdict(self, args):
def __init__(self, root=None, font=None, name=None, exists=False,
**options):
if not root:
root = tkinter._default_root
root = tkinter._get_default_root('use font')
tk = getattr(root, 'tk', root)
if font:
# get actual settings corresponding to the given font
Expand Down Expand Up @@ -184,7 +184,7 @@ def metrics(self, *options, **kw):
def families(root=None, displayof=None):
"Get font families (as a tuple)"
if not root:
root = tkinter._default_root
root = tkinter._get_default_root('use font.families()')
args = ()
if displayof:
args = ('-displayof', displayof)
Expand All @@ -194,7 +194,7 @@ def families(root=None, displayof=None):
def names(root=None):
"Get names of defined fonts (as a tuple)"
if not root:
root = tkinter._default_root
root = tkinter._get_default_root('use font.names()')
return root.tk.splitlist(root.tk.call("font", "names"))


Expand Down
19 changes: 9 additions & 10 deletions Lib/tkinter/simpledialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@
"""

from tkinter import *
from tkinter import messagebox

import tkinter # used at _QueryDialog for tkinter._default_root
from tkinter import messagebox, _get_default_root


class SimpleDialog:
Expand Down Expand Up @@ -128,13 +126,17 @@ def __init__(self, parent, title = None):
title -- the dialog title
'''
Toplevel.__init__(self, parent)
master = parent
if not master:
master = _get_default_root('create dialog window')

Toplevel.__init__(self, master)

self.withdraw() # remain invisible for now
# If the master is not viewable, don't
# If the parent is not viewable, don't
# make the child transient, or else it
# would be opened withdrawn
if parent.winfo_viewable():
if parent is not None and parent.winfo_viewable():
self.transient(parent)

if title:
Expand All @@ -155,7 +157,7 @@ def __init__(self, parent, title = None):

self.protocol("WM_DELETE_WINDOW", self.cancel)

if self.parent is not None:
if parent is not None:
self.geometry("+%d+%d" % (parent.winfo_rootx()+50,
parent.winfo_rooty()+50))

Expand Down Expand Up @@ -259,9 +261,6 @@ def __init__(self, title, prompt,
minvalue = None, maxvalue = None,
parent = None):

if not parent:
parent = tkinter._default_root

self.prompt = prompt
self.minvalue = minvalue
self.maxvalue = maxvalue
Expand Down
27 changes: 27 additions & 0 deletions Lib/tkinter/test/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,33 @@ def tearDown(self):
w.destroy()
self.root.withdraw()


class AbstractDefaultRootTest:

def setUp(self):
self._old_support_default_root = tkinter._support_default_root
destroy_default_root()
tkinter._support_default_root = True
self.wantobjects = tkinter.wantobjects

def tearDown(self):
destroy_default_root()
tkinter._default_root = None
tkinter._support_default_root = self._old_support_default_root

def _test_widget(self, constructor):
# no master passing
x = constructor()
self.assertIsNotNone(tkinter._default_root)
self.assertIs(x.master, tkinter._default_root)
self.assertIs(x.tk, tkinter._default_root.tk)
x.destroy()
destroy_default_root()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, constructor)
self.assertFalse(hasattr(tkinter, '_default_root'))


def destroy_default_root():
if getattr(tkinter, '_default_root', None):
tkinter._default_root.update_idletasks()
Expand Down
34 changes: 32 additions & 2 deletions Lib/tkinter/test/test_tkinter/test_font.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import tkinter
from tkinter import font
from test.support import requires, run_unittest, gc_collect, ALWAYS_EQ
from tkinter.test.support import AbstractTkTest
from tkinter.test.support import AbstractTkTest, AbstractDefaultRootTest

requires('gui')

Expand Down Expand Up @@ -107,7 +107,37 @@ def test_repr(self):
)


tests_gui = (FontTest, )
class DefaultRootTest(AbstractDefaultRootTest, unittest.TestCase):

def test_families(self):
self.assertRaises(RuntimeError, font.families)
root = tkinter.Tk()
families = font.families()
self.assertIsInstance(families, tuple)
self.assertTrue(families)
for family in families:
self.assertIsInstance(family, str)
self.assertTrue(family)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, font.families)

def test_names(self):
self.assertRaises(RuntimeError, font.names)
root = tkinter.Tk()
names = font.names()
self.assertIsInstance(names, tuple)
self.assertTrue(names)
for name in names:
self.assertIsInstance(name, str)
self.assertTrue(name)
self.assertIn(fontname, names)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, font.names)


tests_gui = (FontTest, DefaultRootTest)

if __name__ == "__main__":
run_unittest(*tests_gui)
45 changes: 43 additions & 2 deletions Lib/tkinter/test/test_tkinter/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import tkinter
from test import support
from test.support import os_helper
from tkinter.test.support import AbstractTkTest, requires_tcl
from tkinter.test.support import AbstractTkTest, AbstractDefaultRootTest, requires_tcl

support.requires('gui')

Expand All @@ -20,6 +20,47 @@ def test_image_names(self):
self.assertIsInstance(image_names, tuple)


class DefaultRootTest(AbstractDefaultRootTest, unittest.TestCase):

def test_image_types(self):
self.assertRaises(RuntimeError, tkinter.image_types)
root = tkinter.Tk()
image_types = tkinter.image_types()
self.assertIsInstance(image_types, tuple)
self.assertIn('photo', image_types)
self.assertIn('bitmap', image_types)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.image_types)

def test_image_names(self):
self.assertRaises(RuntimeError, tkinter.image_names)
root = tkinter.Tk()
image_names = tkinter.image_names()
self.assertIsInstance(image_names, tuple)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.image_names)

def test_image_create_bitmap(self):
self.assertRaises(RuntimeError, tkinter.BitmapImage)
root = tkinter.Tk()
image = tkinter.BitmapImage()
self.assertIn(image.name, tkinter.image_names())
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.BitmapImage)

def test_image_create_photo(self):
self.assertRaises(RuntimeError, tkinter.PhotoImage)
root = tkinter.Tk()
image = tkinter.PhotoImage()
self.assertIn(image.name, tkinter.image_names())
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.PhotoImage)


class BitmapImageTest(AbstractTkTest, unittest.TestCase):

@classmethod
Expand Down Expand Up @@ -331,7 +372,7 @@ def test_transparency(self):
self.assertEqual(image.transparency_get(4, 6), False)


tests_gui = (MiscTest, BitmapImageTest, PhotoImageTest,)
tests_gui = (MiscTest, DefaultRootTest, BitmapImageTest, PhotoImageTest,)

if __name__ == "__main__":
support.run_unittest(*tests_gui)
Loading

0 comments on commit 3d569fd

Please sign in to comment.