Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tkinter after_cancel does not behave correctly when called with id=None #77038

Closed
csabella opened this issue Feb 16, 2018 · 11 comments
Closed

tkinter after_cancel does not behave correctly when called with id=None #77038

csabella opened this issue Feb 16, 2018 · 11 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@csabella
Copy link
Contributor

BPO 32857
Nosy @serhiy-storchaka, @csabella, @miss-islington
PRs
  • bpo-32857: tkinter after_cancel to raise error if called with None #5701
  • [3.7] bpo-32857: Raise error when tkinter after_cancel() is called with None. (GH-5701) #5972
  • [3.6] bpo-32857: Raise error when tkinter after_cancel() is called with None. (GH-5701) #5973
  • [2.7] bpo-32857: Raise error when tkinter after_cancel() is called with None (GH-5701) #6620
  • 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:

    assignee = None
    closed_at = <Date 2018-05-05.13:20:35.328>
    created_at = <Date 2018-02-16.13:20:14.204>
    labels = ['3.7', '3.8', 'type-bug', 'expert-tkinter']
    title = 'tkinter after_cancel does not behave correctly when called with id=None'
    updated_at = <Date 2018-05-05.13:20:35.328>
    user = 'https://github.com/csabella'

    bugs.python.org fields:

    activity = <Date 2018-05-05.13:20:35.328>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-05-05.13:20:35.328>
    closer = 'serhiy.storchaka'
    components = ['Tkinter']
    creation = <Date 2018-02-16.13:20:14.204>
    creator = 'cheryl.sabella'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32857
    keywords = ['patch']
    message_count = 11.0
    messages = ['312231', '312233', '312234', '312349', '312384', '312404', '312566', '313207', '313209', '313210', '316208']
    nosy_count = 3.0
    nosy_names = ['serhiy.storchaka', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['5701', '5972', '5973', '6620']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32857'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']

    @csabella
    Copy link
    Contributor Author

    This was discovered while working on bpo-32839. An old comment added for bpo-763637 resulted in further research and it was determined that the comment:

    ! data = self.tk.call('after', 'info', id)
    ! # In Tk 8.3, splitlist returns: (script, type)
    ! # In Tk 8.4, splitlist returns: (script,)

    wasn't correct. The underlying difference is that the call to 'after info' returns different results depending on the way it's called. If it's called with an 'id', it will return a tuple of (script, type) if the id is valid or a TclError if the id isn't valid. However, if id is None, then 'after info' returns a tuple of all the event ids for the widget. In the case of the original bug in bpo-763637, the reported message shows the return value of ('after#53',), which is definitely an after event id and not a (script,) tuple.

    Serhiy mentions on bpo-32839 that the current code also deletes the script for the first event if after_cancel is called with None.
    https://bugs.python.org/issue32839#msg312199

    @csabella csabella added 3.8 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error labels Feb 16, 2018
    @csabella
    Copy link
    Contributor Author

    I can open a PR for this, but thought it might be best to discuss it first as far as what should happen with None.

    1. Raise an error that it's an incorrect call.
    2. Ignore it and just return.
    3. Loop through all the events and after_cancel each one.

    Tcl allows the after_cancel call to be an id or script(s), but they don't have functionality that would cancel everything (unless all the script values were sent).

    FWIW, if no parameters are sent, this is the Tcl code:
    	if (objc < 3) {
    	    Tcl_WrongNumArgs(interp, 2, objv, "id|command");
    	    return TCL_ERROR;
    	}

    @serhiy-storchaka
    Copy link
    Member

    I prefer the option 1. Even if calling after_cancel(None) would make sense, currently it silently does wrong things, and it is more errorproof to make it raising an exception.

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Feb 16, 2018
    @serhiy-storchaka
    Copy link
    Member

    I have noticed that there are no tests for after() and after_cancel(). Maybe first write tests for them and later add a test for this specific issue to them?

    @csabella
    Copy link
    Contributor Author

    I had also noticed that the after commands didn't have tests. Did you want me to add tests in a separate issue before this one is merged or did you want me to add tests under this PR?

    Thanks!

    @serhiy-storchaka
    Copy link
    Member

    It's up to you.

    @csabella
    Copy link
    Contributor Author

    I've added the tests for after and after_idle. I used update and update_idletasks to make sure they processed. Even scheduling an after event for 0 ms didn't guarantee it would process, but I wasn't sure if there was a better way besides update to get it to process?

    @serhiy-storchaka
    Copy link
    Member

    New changeset 74382a3 by Serhiy Storchaka (Cheryl Sabella) in branch 'master':
    bpo-32857: Raise error when tkinter after_cancel() is called with None. (GH-5701)
    74382a3

    @miss-islington
    Copy link
    Contributor

    New changeset 73a4396 by Miss Islington (bot) in branch '3.6':
    bpo-32857: Raise error when tkinter after_cancel() is called with None. (GH-5701)
    73a4396

    @miss-islington
    Copy link
    Contributor

    New changeset a5303dd by Miss Islington (bot) in branch '3.7':
    bpo-32857: Raise error when tkinter after_cancel() is called with None. (GH-5701)
    a5303dd

    @serhiy-storchaka
    Copy link
    Member

    New changeset 3a04598 by Serhiy Storchaka (Cheryl Sabella) in branch '2.7':
    bpo-32857: Raise error when tkinter after_cancel() is called with None. (GH-5701) (GH-6620)
    3a04598

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants