python / cpython

The Python programming language
https://www.python.org
Other
62.21k stars 29.89k forks source link

tkinter focus_get() with non-tkinter Tk widget #88758

Open 98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 opened 3 years ago

98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago
BPO 44592
Nosy @terryjreedy, @serhiy-storchaka, @Akuli, @E-Paine
Superseder
  • bpo-734176: Make Tkinter.py's nametowidget work with cloned menu widgets
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'expert-tkinter', '3.9', '3.10', '3.11'] title = 'tkinter focus_get() with non-tkinter Tk widget' updated_at = user = 'https://github.com/Akuli' ``` bugs.python.org fields: ```python activity = actor = 'Akuli' assignee = 'none' closed = False closed_date = None closer = None components = ['Tkinter'] creation = creator = 'Akuli' dependencies = [] files = [] hgrepos = [] issue_num = 44592 keywords = [] message_count = 12.0 messages = ['397199', '397200', '397211', '397213', '397216', '397217', '397220', '397222', '397224', '397681', '397726', '397727'] nosy_count = 4.0 nosy_names = ['terry.reedy', 'serhiy.storchaka', 'Akuli', 'epaine'] pr_nums = [] priority = 'normal' resolution = None stage = 'resolved' status = 'open' superseder = '734176' type = 'behavior' url = 'https://bugs.python.org/issue44592' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago

    The purpose of focus_get() is to return the widget that currently has the focus. It tries to convert its result to a tkinter widget, which can fail, because not all Tk widgets are known to tkinter. Consider this, for example:

        import tkinter
    
        def print_focused_widget():
            print(repr(root.focus_get()))
            root.after(1000, print_focused_widget)
    
        root = tkinter.Tk()
        menu = root["menu"] = tkinter.Menu()
        menu.add_cascade(label="Click here", menu=tkinter.Menu())
        print_focused_widget()
        tkinter.mainloop()

    Output, with menu clicked after a couple seconds (on Linux):

        None
        <tkinter.Tk object .>
        <tkinter.Tk object .>
        Exception in Tkinter callback
        Traceback (most recent call last):
          File "/home/akuli/.local/lib/python3.10/tkinter/__init__.py", line 1916, in __call__
            return self.func(*args)
          File "/home/akuli/.local/lib/python3.10/tkinter/__init__.py", line 838, in callit
            func(*args)
          File "/home/akuli/porcu/foo.py", line 4, in print_focused_widget
            print(repr(root.focus_get()))
          File "/home/akuli/.local/lib/python3.10/tkinter/__init__.py", line 782, in focus_get
            return self._nametowidget(name)
          File "/home/akuli/.local/lib/python3.10/tkinter/__init__.py", line 1531, in nametowidget
            w = w.children[n]
        KeyError: '#!menu'

    Some nametowidget() calls in tkinter/init.py already handle this correctly. Consider winfo_children(), for example:

            try:
                # Tcl sometimes returns extra windows, e.g. for
                # menus; those need to be skipped
                result.append(self._nametowidget(child))
            except KeyError:
                pass
    98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago

    Forgot to mention: The correct fix IMO would be to return None when a KeyError occurs. This way code like focus_get() == some_tkinter_widget would always do the right thing, for example.

    919475b3-36a2-4cd7-997c-9c38f05f93c7 commented 3 years ago

    I agree with Akuli that raising a KeyError is not expected behaviour (combined with the fact this is caught elsewhere), and therefore is probably a regression.

    While we could use winfo class to determine the type of Tk widget, this would probably require a reasonably sized refactor of tkinter (and we would still need to support cases when it's a type we don't know). Therefore, I think returning None is the best solution.

    Akuli, would you like to create a pull request for this?

    919475b3-36a2-4cd7-997c-9c38f05f93c7 commented 3 years ago

    Sorry, I should specify that we would use winfo class in order to then return a new tkinter object for the existing Tk widget (which currently cannot be done)

    serhiy-storchaka commented 3 years ago

    It is a duplicate of bpo-734176.

    98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago

    I found bpo-734176 before I created this. It is NOT a duplicate. While bpo-734176 is about menus, this one is about focus_get(), and not necessarily related to menus. In fact, I initially noticed this with an "open file" dialog, not with a menu.

    I'm not putting my address into your CLA, thank you very much.

    terryjreedy commented 3 years ago

    Akuli, what tk widgets do you think are not known to tkinter? In any case, tk menu is known to tkinter.

    I cannot reproduce when running on Windows with 3.10.0b3: Add "print(root.children)" (after add_cascade) results in {'!menu': \<tkinter.Menu object .!menu>, '!menu2': \<tkinter.Menu object .!menu2>}. The names are created in tkinter.py lines 2564-2573.

    I then see 'None' once and then '\<tkinter.Tk object .>' indefinitely even while hovering over and clicking 'click me' and the dropdown. If I click outside the tk box, the print returns to 'None'.

    Maybe there is an OS difference in what is considered to have 'focus'.

    Key '#!menu' looks like '!menu' with '#' prepended. Someone could try changing the tkinter code referenced above and see if the change appears in the bad key. Also check the contents of root.children.

    terryjreedy commented 3 years ago

    I am not quite convinced that this is a duplicate of bpo-734176. The latter is about tearoff clones and nothing is cloned here. But I do notice that number 'names were also prefixed with '#'. What happens if 'tearoff=0' is added to the cascade so that it is not even clonable. The tkinter naming of instances after the class was added less than 10 years ago.

    98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago

    Unfortunately I don't know any real-world examples of this on Windows. The open file dialog works very differently on Windows: it uses the native Windows dialog, whereas on Linux, it's implemented in Tcl.

    Meanwhile, here's a platform-independent toy example:

        import tkinter
    
        root = tkinter.Tk()
        root.tk.eval("""
        entry .e
        pack .e
        focus .e
        """)
        root.after(500, root.focus_get)
        root.mainloop()

    Also, thanks for reopening!

    terryjreedy commented 3 years ago
    Traceback (most recent call last):
      File "C:\Programs\Python310\lib\tkinter\__init__.py", line 1921, in __call__
        return self.func(*args)
      File "C:\Programs\Python310\lib\tkinter\__init__.py", line 839, in callit
        func(*args)
      File "C:\Programs\Python310\lib\tkinter\__init__.py", line 783, in focus_get
        return self._nametowidget(name)
      File "C:\Programs\Python310\lib\tkinter\__init__.py", line 1536, in nametowidget
        w = w.children[n]
    KeyError: 'e'

    Is catching KeyError in the following try: # Tcl sometimes returns extra windows, e.g. for # menus; those need to be skipped result.append(self._nametowidget(child)) except KeyError: pass really correct? It appears to skip things that *can* get focus by key or mouse action. But what choice is there?

    Silently failing when asked to focus on something is even less obviously correct. For 'widget = root.focus_get' to assign None to widget is not obviously useful as it likely just delays the error.

    98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago

    Here are the options:

    98e2c05b-e10f-4e0f-bfa1-526da4fe30f0 commented 3 years ago

    Typo in previous message: I meant widget.focus_get() is None. It currently means "this application doesn't have focus", while is not None currently means "this application has focus".

    TheLizzard commented 11 months ago

    I ran into this problem with this code:

    from tkinter.filedialog import askopenfilename
    import tkinter as tk
    
    def f(e):
        print(root.focus_get())
    
    root = tk.Tk()
    root.bind("<FocusOut>", f)
    
    button = tk.Button(root, command=askopenfilename, text="click me")
    button.pack()
    
    root.mainloop()

    The code raises a KeyError on Ubuntu 22.04 but not on Windows 11.

    Using this to investigate:

    widgets = set()
    def g():
        widget = root.tk.call('focus')
        widgets.add(str(widget))
        root.after(10, g)
    g()

    I get that on Windows, tcl doesn't even try to name the widgets inside the dialogbox but on Ubuntu:

    >>> widgets
    {'', '.__tk_filedialog.contents.f2.cancel', '.', '.__tk_filedialog.contents.f2.ent', '.__tk_filedialog'}