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

[csabella]

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]

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]

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]

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]

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]

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]

It's up to you.

[csabella]

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]

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]

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]

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]

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