Tk Source Code

Current implementation of event binding (tkBind.c) has some bugs, this includes a design bug, the idea with the event ring is (in general) not working. See the following problems:

1) Sometimes double-click with mouse will not be detected, nothing happens although this event is bound.

2) Immediately after startup of application Scidb (same with Scid, Scid vs PC) it's not possible to open the menu via a shortcut like <Alt-M>, this event will be gobbled.

3) If a statement like "event generate . <1>" is executed, and after some time (> 500 ms) this statement is executed again, then it may happen that a double-click event will be triggered, though only single-click events are expected, because the triggering of double-click events has to fit time requirements (due to manual).

4) It's possible to bind an event like <Quadruple-1>, but it's nearly impossible to trigger this event (with mousepad). Even a triple-click is not so easy. This behavior is user-unfriendly.

5) "event generate . <FocusIn> -sendevent 1" is not working, the argument of sendevent will be lost.

Forgot to close this one, TIP#532 got voted YES and merged on 2019-08-28

TIP #532 now published for review.

I'm also unlucky with the change of the return type of function Tk_CreateBinding, because the X11 headers are not using type 'Mask', although it is defined. New version [6377cb0d76] is reverting this change, so Tk_CreateBinding is still returning 'unsigned long'. Inside the implementation now I'm using the locally defined types 'ModMask' and 'EventMask', this makes the code more clear.

I have problems with adding new TIP, because

fossil push http://[email protected]/tips

is asking for the password, but is not accepting my password, always the message "Error: login failed".

On Windows, bind.test still passes 100 %:

Total	523	Passed	520	Skipped	3	Failed	0

About the change in the return value of Tk_CreateBinding I'm very slightly uncomfortable because this function is part of the public interface of Tk. Even if binary compatible this change should be clearly mentioned in the TIP I think. Given that "Mask" is typedef'ed in xlib/X11/X.h I believe it will be fine.

I've checked in [ec38f729fa]:

1. Return type of Tk_CreateBinding changed to 'Mask', because the return value is am event mask. This change is binary compatible, because 'Mask' is an alias for 'unsigned long' (old return type).

2. Issue with homogeneous equal sequences fixed, see this script:

bind . <1><Control-1> { lappend x "first" }
bind . <Control-1><1> { lappend x "last" }
event generate . <Control-1>
event generate . <Control-1>
set x

This script should return "last" due to manual. Legacy implementation is failing this test case. See new test cases bind-33.12 and bind-33.13.

I'm about to finish the TIP, will come soon.

Please do a test under Windows.

I'm glad that this is the final version. Many thanks for your patience.

> How come that there was a problem on Windows only, and not on Linux or macOS? Isn't it the case that only Windows revealed this problem, but it was present on all platforms? The counters need to be reset upon window deletion, regardless of the platform, isn't it?

On Linux/MacOS the window ID will not be re-used after destroying a window, so the counter must not be reset, the new unique window ID is causing an implicit reset. This means that only the Windows version can expose this problem (in this way), because Windows is re-using expired window ID's.

This behaviour shows a difference to old implementation: here the traversal of the event loop is stopping at the right time, when a create notification will be encountered. But the overflow problem is (in general) not solvable with old implementation.

Now I will write the TIP.

This time I have good news: bind.test passes 100% OK on Windows Vista, Linux Debian 8 and macOS 10.14 Mojave, with the bugfix branch. Congratulations!

So the problem was not due to some obscure compiler specificity. How come that there was a problem on Windows only, and not on Linux or macOS? Isn't it the case that only Windows revealed this problem, but it was present on all platforms? The counters need to be reset upon window deletion, regardless of the platform, isn't it?

The last remaining thing before merging is to approve a TIP. Keep up the good work, we're close to the end! Thanks!

Your test gives me an unexpected result: when TkBindDeadWindow is called, the window ID is not matching (see line 1397), but later when matching events the window ID is matching (see line 2263). Thus the mechanism with TkBindDeadWindow does not work, this functions comes too late, the window ID is already out-of-date here. So I realized the reset of the counters with an alternative way: now I use the DestroyNotify event. Please try version [1fd96a3c2f].

With added fflush(stdout) after each debug printf, here is the relevant output I get for TESTFLAGS="-file bind.test -match * -verbose bepst" (excerpt):

[...]
---- bind-15.21 start
++++ bind-15.21 PASSED
---- bind-15.22 start
Tk_BindEvent(.t.f): 1 1
MatchPatterns(1): 2 1
Tk_BindEvent(.t.f): 2 2
MatchPatterns(1): 2 2
TkBindDeadWindow(.t.f)
Tk_DeleteAllBindings(.t.f): 68
++++ bind-15.22 PASSED
---- bind-15.23 start
###### bind-15.23 ##########################################
Tk_BindEvent(.t.f): 3 3
MatchPatterns(1): 2 3
Tk_BindEvent(.t.f): 1 1
MatchPatterns(1): 2 1
############################################################
TkBindDeadWindow(.t.f)
Tk_DeleteAllBindings(.t.f): 69


==== bind-15.23 MatchPatterns procedure, time wrap-around FAILED
==== Contents of test case:

puts "###### bind-15.23 ##########################################"
    bind .t.f <Double-1> {set x 1}
    set x 0
    event generate .t.f <Button-1> -time -100
    event generate .t.f <Button-1> -time 500
    event generate .t.f <ButtonRelease-1>
puts "############################################################"
    return $x

---- Result was:
1
---- Result should have been (exact matching):
0
==== bind-15.23 FAILED

---- bind-15.24 start
++++ bind-15.24 PASSED
[...]

Yes, the window destruction is important between the tests since we believe that the reason why bind-15.23 passes or fails is linked to what tests were previously run, and on the hypothesis that something is not properly erased when the window to which the scripts are bound gets destroyed.

Perhaps an important observation: With your modified script (the one dated "2019-01-01 10:45:03"), all output x values are the expected ones (i.e. 0, 1, 0) in interactive mode, that is when I paste the script in tclsh86tg compiled at [2bbced9e]. However, when I source this same script into that same tclsh86tg, then I get the wrong output (i.e. 0, 1, 1)

I'm attaching the output for the bind.test with TESTFLAGS="-file bind.test -match * -verbose beps". I guess that's what you'd like to get from me because of those 68/69 constants in [2bbced9e], and since there is no debug output when running, say, TESTFLAGS="-file bind.test -match bind-15.2[123]"

Final note: I suggest to add a fflush statement after each printf, so that the debug output would be located at its correct place in the output flow.

About the test script of Francois, added 2018-12-22 12:58:44. If you read it carefully you will see that the result of third test must be "1", not "0". This test script has to be rewritten ("destroy .f" is important; for clearness I removed superfluous lines of code):

package require Tk

# bind-15.21
    pack [frame .f]; update
    bind .f <Double-1> {set x 1}
    set x 0
    event generate .f <Button-1> -time 300
    event generate .f <Button-1> -time 900
    destroy .f
set x ; # expected 0

# bind-15.22
    pack [frame .f]; update
    bind .f <Double-1> {set x 1}
    set x 0
    event generate .f <Button-1> -time -100
    event generate .f <Button-1> -time 200
    destroy .f
set x ; # expected 1

# bind-15.23
    pack [frame .f]; update
    bind .f <Double-1> {set x 1}
    set x 0
    event generate .f <Button-1> -time -100
    event generate .f <Button-1> -time 500
    destroy .f

set x   ; # expected 0, but it's 1

win7-64, msys2/mingw gcc version 8.2.1
Same as before for msys2.  Passes 15.23, Francois' test script fails.

$ make test TESTFLAGS="-file bind.test -verbose buvpts -match bind-15.2[123]"
Tests running in interp:  C:/msys64/home/bll/t/tcl-work/tk/win/tktest.exe
Tests located in:  C:/msys64/home/bll/t/tcl-work/tk/tests
Tests running in:  C:/msys64/home/bll/t/tcl-work/tk/win
Temporary files stored in C:/msys64/home/bll/t/tcl-work/tk/win
Test files sourced into current interpreter
Running tests that match:  {bind-15.2[123]}
Skipping test files that match:  l.*.test
Only running test files that match:  bind.test
Tests began at Mon Dec 31 18:29:00 -0800 2018
bind.test
---- bind-15.21 start
++++ bind-15.21 took 2671 μs
++++ bind-15.21 PASSED
---- bind-15.22 start
++++ bind-15.22 took 631 μs
++++ bind-15.22 PASSED
---- bind-15.23 start
###### bind-15.23 ##########################################
############################################################
++++ bind-15.23 took 1818 μs
++++ bind-15.23 PASSED

Tests ended at Mon Dec 31 18:29:01 -0800 2018
all.tcl:        Total   518     Passed  3       Skipped 515     Failed  0
Sourced 1 Test Files.
stderr32Tests running in interp:  C:/msys64/home/bll/t/tcl-work/tk/win/tktest.exe
Tests located in:  C:/msys64/home/bll/t/tcl-work/tk/tests/ttk
Tests running in:  C:/msys64/home/bll/t/tcl-work/tk/win
Temporary files stored in C:/msys64/home/bll/t/tcl-work/tk/win
Test files sourced into current interpreter
Running tests that match:  {bind-15.2[123]}
Skipping test files that match:  l.*.test
Only running test files that match:  bind.test
Tests began at Mon Dec 31 18:29:02 -0800 2018
Error:  No test files remain after applying your match and skip patterns!

Tests ended at Mon Dec 31 18:29:02 -0800 2018
all.tcl:        Total   0       Passed  0       Skipped 0       Failed  0
Sourced 0 Test Files.
stderr32

bll@bll-win7-64 MINGW64 ~/t/tcl-work/tk/win
$ $HOME/t/local-test/bin/tclsh86.exe  t.tcl
0=0
1=1
0=1

I'm sorry, my commit was incomplete, version [c8fdf4216b] should compile and link properly.

Something got lost.
I'm getting an undefined reference to PhysOwned_Contains.
Cannot link on windows or linux.
More fossil issues?

It's difficult to understand, what is going on. Based on tests under Linux the double button event cannot be triggered. I have pushed a new test version, please try [2bbced9e17] with tests/bind.test, this version is tracing the repetition counters, as well as the reset of the counters. It's interesting that this bug occurs when compiled with VC, but not when compiled with mingw.

The tests pass on win7-64 with mingw. 
Rather odd that it is different with the two compilers.
(with [e5db5761f4] ).

Though Francois's test script still does have the expected 0, but is 1 issue.

At [e78437d5aa] all tests from bind.test PASS for me on Windows, except bind-15.23 (plus three that are skipped because they have "nonPortable" constraint). This includes the two new tests bind-32.14 and bind-32.15, they both PASS. Your fixes seem to be correct, but they still appear to miss some aspect of the things.

I don't understand this. I've pushed [e78437d5aa], with two new test cases. What's about bind-32.15, is this test case passing?

Hmpf. Still the same unfortunately.

All the time I was a bit blind. I did reset the key counters, but not the button counters. Please try [e5db5761f4], this should be the solution.

Thanks for info about writing TIP, I will do this soon.

Unfortunately bind-15.23 is still failing. Probably something else should be reset (how can I help in finding what this is?)

Regarding the TIP, please start here: https://core.tcl-lang.org/tips/doc/trunk/index.md

TIPs are stored in a fossil repository. The "Getting started" help page is here: https://core.tcl-lang.org/tips/doc/trunk/doc/help.md

Okay, I see, the promoted events has to be update when a windows will be destroyed. Realized with commit of [1f1f74886a]. I hope that bind-15.23 will pass now.

I will try to write a TIP. Please, Francois, can you help me, on which site I can find the TIP editor. I have forgotten this, it's a long time ago with last TIP.

For me the output for the tests with TESTFLAGS="-file bind.test -match bind-15.2[123]" is slightly different:

HandleEventGenerate: 300 (300)
MatchEventNearby: 300, 0 (300, 0)
TestNearbyTime: 300, 0 => 300
HandleEventGenerate: 900 (900)
MatchEventNearby: 900, 300 (900, 300)
TestNearbyTime: 900, 300 => 600
MatchEventNearby: 0, 0 (0, 0)
TestNearbyTime: 0, 0 => 0
HandleEventGenerate: 4294967196 (-100)
HandleEventGenerate: 200 (200)
MatchEventNearby: 200, 4294967196 (200, -100)
TestNearbyTime: 200, -100 => 300
HandleEventGenerate: 4294967196 (-100)
MatchEventNearby: 4294967196, 200 (-100, 200)
TestNearbyTime: -100, 200 => 300
HandleEventGenerate: 500 (500)
MatchEventNearby: 500, 4294967196 (500, -100)
TestNearbyTime: 500, -100 => 600
MatchEventNearby: 0, 0 (0, 0)
TestNearbyTime: 0, 0 => 0

Right, I misread the -match line you specified.
The test script you supplied does indeed exhibit the characteristics that
you specify:

$ $HOME/t/local-test/bin/tclsh86.exe  t.tcl
0=0
1=1
0=1
HandleEventGenerate: 300 (300)
MatchEventNearby: 300, 0 (300, 0)
TestNearbyTime: 300, 0 => 300
HandleEventGenerate: 900 (900)
MatchEventNearby: 900, 300 (900, 300)
TestNearbyTime: 900, 300 => 600
MatchEventNearby: 0, 0 (0, 0)
TestNearbyTime: 0, 0 => 0
HandleEventGenerate: 4294967196 (-100)
MatchEventNearby: 4294967196, 900 (-100, 900)
TestNearbyTime: -100, 900 => 1000
HandleEventGenerate: 200 (200)
MatchEventNearby: 200, 4294967196 (200, -100)
TestNearbyTime: 200, -100 => 300
MatchEventNearby: 0, 0 (0, 0)
TestNearbyTime: 0, 0 => 0
HandleEventGenerate: 4294967196 (-100)
MatchEventNearby: 4294967196, 200 (-100, 200)
TestNearbyTime: -100, 200 => 300
HandleEventGenerate: 500 (500)
MatchEventNearby: 500, 4294967196 (500, -100)
TestNearbyTime: 500, -100 => 600
MatchEventNearby: 0, 0 (0, 0)
TestNearbyTime: 0, 0 => 0


The regular tests:

$ make test TESTFLAGS="-file bind.test -verbose buvpts -match bind-15.2[123]"
Tests running in interp:  C:/msys64/home/bll/t/tcl-work/tk/win/tktest.exe
Tests located in:  C:/msys64/home/bll/t/tcl-work/tk/tests
Tests running in:  C:/msys64/home/bll/t/tcl-work/tk/win
Temporary files stored in C:/msys64/home/bll/t/tcl-work/tk/win
Test files sourced into current interpreter
Running tests that match:  {bind-15.2[123]}
Skipping test files that match:  l.*.test
Only running test files that match:  bind.test
Tests began at Sat Dec 29 15:36:11 -0800 2018
bind.test
---- bind-15.21 start
++++ bind-15.21 took 2342 μs
++++ bind-15.21 PASSED
---- bind-15.22 start
++++ bind-15.22 took 2030 μs
++++ bind-15.22 PASSED
---- bind-15.23 start
++++ bind-15.23 took 1648 μs
++++ bind-15.23 PASSED

Tests ended at Sat Dec 29 15:36:12 -0800 2018
all.tcl:        Total   516     Passed  3       Skipped 513     Failed  0
Sourced 1 Test Files.
HandleEventGenerate: 300 (300)
MatchEventNearby: 300, 0 (300, 0)
TestNearbyTime: 300, 0 => 300
HandleEventGenerate: 900 (900)
MatchEventNearby: 900, 300 (900, 300)
TestNearbyTime: 900, 300 => 600
MatchEventNearby: 0, 0 (0, 0)
TestNearbyTime: 0, 0 => 0
HandleEventGenerate: 4294967196 (-100)
HandleEventGenerate: 200 (200)
MatchEventNearby: 200, 4294967196 (200, -100)
TestNearbyTime: 200, -100 => 300
HandleEventGenerate: 4294967196 (-100)
HandleEventGenerate: 500 (500)
MatchEventNearby: 500, 4294967196 (500, -100)
TestNearbyTime: 500, -100 => 600
stderr32Tests running in interp:  C:/msys64/home/bll/t/tcl-work/tk/win/tktest.exe
Tests located in:  C:/msys64/home/bll/t/tcl-work/tk/tests/ttk
Tests running in:  C:/msys64/home/bll/t/tcl-work/tk/win
Temporary files stored in C:/msys64/home/bll/t/tcl-work/tk/win
Test files sourced into current interpreter
Running tests that match:  {bind-15.2[123]}
Skipping test files that match:  l.*.test
Only running test files that match:  bind.test
Tests began at Sat Dec 29 15:36:13 -0800 2018
Error:  No test files remain after applying your match and skip patterns!

Tests ended at Sat Dec 29 15:36:13 -0800 2018
all.tcl:        Total   0       Passed  0       Skipped 0       Failed  0
Sourced 0 Test Files.
stderr32

About bind-15.23 failure:

- x becomes 1 as soon as this line is executed.

- bind-15.23 passes if I add bind .t.f <Double-1> {} in the -cleanup section of both bind-15.21 and bind-15.22

Conclusion: This is definitely again the same problem as with bind-32.10: something is not properly reset upon window destruction. There is one single issue and [0f6871d942] fixed it only partially.

My results at [4e8f2bc60e]:

% source gregorstest.tcl
HandleEventGenerate: 4294967196 (-100)
HandleEventGenerate: 500 (500)
MatchEventNearby: 500, 4294967196 (500, -100)
TestNearbyTime: 500, -100 => 600
success
%

But bind-15.23 still fails (note to Brad: you didn't try the right test: bind-15.23 is not run when you -match bind-15.[123]). bind-15.23 is also failing at the tip of the bugfix branch, i.e. at [0420e1fa].

Did you see what I wrote about this failure on "2018-12-22 12:58:44"? bind-15.23 only fails in specific conditions, and I have provided a sample isolated script in that contribution below.

Finally: about your proposal for an extended syntax I would say this needs a TIP, yes. It's a new feature. It could be done in trunk (8.7) only, but perhaps not in core-8-6-branch. Anyway, IMO the changes you have proposed to fix the present ticket are more than large enough to deserve being explained in a TIP. I think that the SUPPORT_ADDITIONAL_MOTION_SYNTAX feature is the only thing that changes from the user point of view compared to the legacy code, but I'd like to see this written somewhere.

Further than that, I think all the changes should be clearly and carefully described in a TIP. We have some points to discuss on tcl-core I think (for instance the PREFER_MOST_SPECIALIZED_EVENT thing, or why the expected result for bind-22.92 was changed from the legacy "43" to the revised "1" without any change in the test content, or whether you finally considered item 4 of my answer dated "2018-10-28 20:26:57", and probably a handful of other points.)

Well, seems to work building with mingw.  Francois uses VC though, right?

bll@bll-win7-64 MINGW64 ~/t/tcl-work
$ $HOME/t/local-test/bin/wish86.exe t.tcl
success
HandleEventGenerate: 4294967196 (-100)
HandleEventGenerate: 500 (500)
MatchEventNearby: 500, 4294967196 (500, -100)
TestNearbyTime: 500, -100 => 600

bll@bll-win7-64 MINGW64 ~/t/tcl-work
$ uname -a
MINGW64_NT-6.1 bll-win7-64 2.10.0(0.325/5/3) 2018-02-09 15:25 x86_64 Msys

bll@bll-win7-64 MINGW64 ~/t/tcl-work
$ gcc --version
gcc.exe (Rev1, Built by MSYS2 project) 8.2.0

---- bind-15.23 start
++++ bind-15.23 took 1718 μs
++++ bind-15.23 PASSED
...
all.tcl:        Total   516     Passed  513     Skipped 3       Failed  0



$ make test TESTFLAGS="-file bind.test -verbose buvpts -match bind-15.[123]"
Tests running in interp:  C:/msys64/home/bll/t/tcl-work/tk/win/tktest.exe
Tests located in:  C:/msys64/home/bll/t/tcl-work/tk/tests
Tests running in:  C:/msys64/home/bll/t/tcl-work/tk/win
Temporary files stored in C:/msys64/home/bll/t/tcl-work/tk/win
Test files sourced into current interpreter
Running tests that match:  {bind-15.[123]}
Skipping test files that match:  l.*.test
Only running test files that match:  bind.test
Tests began at Mon Dec 24 12:32:22 -0800 2018
bind.test
---- bind-15.1 start
++++ bind-15.1 took 2093 μs
++++ bind-15.1 PASSED
---- bind-15.2 start
++++ bind-15.2 took 1471 μs
++++ bind-15.2 PASSED
---- bind-15.3 start
++++ bind-15.3 took 1432 μs
++++ bind-15.3 PASSED

Tests ended at Mon Dec 24 12:32:22 -0800 2018
all.tcl:        Total   516     Passed  3       Skipped 513     Failed  0
Sourced 1 Test Files.
TestNearbyTime: 0, 0 => 0
TestNearbyTime: 0, 0 => 0
TestNearbyTime: 0, 0 => 0
TestNearbyTime: 0, 0 => 0
TestNearbyTime: 0, 0 => 0
stderr32Tests running in interp:  C:/msys64/home/bll/t/tcl-work/tk/win/tktest.exe
Tests located in:  C:/msys64/home/bll/t/tcl-work/tk/tests/ttk
Tests running in:  C:/msys64/home/bll/t/tcl-work/tk/win
Temporary files stored in C:/msys64/home/bll/t/tcl-work/tk/win
Test files sourced into current interpreter
Running tests that match:  {bind-15.[123]}
Skipping test files that match:  l.*.test
Only running test files that match:  bind.test
Tests began at Mon Dec 24 12:32:23 -0800 2018
Error:  No test files remain after applying your match and skip patterns!

Tests ended at Mon Dec 24 12:32:23 -0800 2018
all.tcl:        Total   0       Passed  0       Skipped 0       Failed  0
Sourced 0 Test Files.
stderr32

I've committed [4e8f2bc60e], this is a test version for bind-15.23. Please test the following script separately:

    pack [frame .f]; focus -force .f; update
    bind .f <Double-1> { set x "failed" }
    set x "success"
    event generate .f <Button-1> -time -100
    event generate .f <Button-1> -time 500
    event generate .f <ButtonRelease-1>
    puts $x
    exit

and send me the trace, I hope that this information is sufficient for finding the problem.

Last pushed version is passing all test cases under Linux/Mac, so this special Windoze problem is last one.

What's about my suggestion:

Due to documentation it's possible to combine motion events with buttons, for example <B2-Motion>. For any reason the syntax <Motion-2> is not allowed, but I think it makes sense to allow the latter syntax. And in this way it should be allowed to express <B1-B2-Motion> with <Motion-1-2>. I've already implemented this suggestion, but it is commented out (not yet active, see compiler constant SUPPORT_ADDITIONAL_MOTION_SYNTAX).

Does this need a TIP?

Definitely an improvement (but I had to commit [4499a2c5ad] to make the branch build on Windows).

Now only bind-15.23 is still failing (identically). So there must be yet another issue.

Yes, I think you're right, there is no clash. I have overseen the second possibility, when the state is not resetting after destroying the window. This seems to be the case here, see the following code:

pack [frame .t.f]
focus .t.f
event generate .t.f <KeyPress-A>
destroy .t.f
pack [frame .t.f]
focus .t.f
event generate .t.f <KeyPress-A>

It seems that newly created window has same window ID (under Windoze; under Linux this is not the case), so the detection of double key events don't works properly. Please try version [0f6871d942]. (NOTE: fossil gave the message "server says: 307 Temporary Redirect" after first push, so I did a second empty push, because w/o the 2nd the 1st wasn't visible in repository.)

I've replaced test case 32.10, the old has become superfluous, and the new one is testing whether the states after destroying the window are clean.

Moreover I've added test cases 32.12 and 23.13 for more safety.

More on bind-32.10 on Windows, in the bugfix branch:

Look at the below scripts, it illustrates dependency of result of bind-32.10 on results from previously run tests such as bind-32.9:

# Script 1: run only bind-32.10
#   --> result from bind-32.10 IS the expected result

package require Tk

    focus -force .
    update
    set x {}

# bind-32.10
    bind . <1> { set x "Single" }
    bind . <Double-1> { set x "Double" }
    event generate . <1>
    event generate . <KeyPress> -keycode 20
    event generate . <1>
    set x    ; # expected "Single" (and obtained)


# Script 2: run bind-32.9 and then bind-32.10
#   --> result from bind-32.10 is NOT the expected result

package require Tk

    focus -force .
    update
    set x {}

# bind-32.9
    bind . <Any-Key> { set x "Key" }
    event generate . <KeyPress> -keysym Caps_Lock
    set x     ; # expected "Key" (and obtained)

# bind-32.10
    bind . <1> { set x "Single" }
    bind . <Double-1> { set x "Double" }
    event generate . <1>
    event generate . <KeyPress> -keycode 20
    event generate . <1>
    set x    ; # expected "Single" BUT obtained "Double"

In the bugfix branch, on Windows, bind-32.10 does not fail when run in isolation, but it fails when running -match bind-32.*.

I don't believe at all in that odd clashing story. The issue with bind-32.10 is the same as with bind-15.23. There is only one single cause, exposed by two different tests. Fixing one of these two tests will fix the other one in the same move.

On Windows:

Test case bind-15.23 does not fail when run in isolation,i.e. when running -matches bind-15.23. It fails when running -matches bind-15.2[123] but not when running -matches bind-15.2[23] or -matches bind-15.2[13].

And indeed I can reproduce in tclsh in branch bug6e8afe516d:

package require Tk

focus -force .
update
bind . <Double-1> {set x 1}

# bind-15.21
    set x 0
    event generate . <Button-2> 
    event generate . <ButtonRelease-2>
    event generate . <Button-1> -time 300
    event generate . <Button-1> -time 900
    event generate . <ButtonRelease-1>
set x ; # expected 0

# bind-15.22
    set x 0
    event generate . <Button-1> -time -100
    event generate . <Button-1> -time 200
    event generate . <ButtonRelease-1>
set x ; # expected 1

# bind-15.23
    set x 0
    event generate . <Button-1> -time -100
    event generate . <Button-1> -time 500
    event generate . <ButtonRelease-1>

set x   ; # expected 0, but it's 1

However, I get "1" at the end of this script also in core-8-6-branch, whereas -matches bind-15.2[123] passes in that branch (with "0" expected and obtained in bind-15.23). This makes me think that the new implementation would be correct and that the test would be incorrectly written.

About bind-32.10 I have not yet made my mind.

I've fixed test case bind-32.9 in [a3129f7552], now it should pass.

bind-32.10 is failing, this is proving my assumption that there is a clash between modifier keys and normal keys (under Windows). What should we do, a general bugfix (exchanging the values of dispPtr->numModKeyCodes, such that there is no clash anymore), or a work-around in tkBind.c, trying to distinguish between modifier keys and normal keys by checking the change of the state? I guess that bind-32.10 is also failing with current version of wish, so this problem is not new, only the detection of this problem is new.

Test case bind-15.23 is still open, I have to build a special test version for this, because under Linux this problem is not reproducible. (I guess that it's only a compiler dependent problem with negative time values.)

The Shift_Lock keysym is not defined in Tk for Windows (see the keymap at the beginning of tkWinKey.c). On Windows the keysym that corresponds to the VK_CAPITAL key code (0x20) is Caps_lock, which conveys a different meaning than Shift_Lock, and is consistent with the Microsoft documentation.

Bind-32.9 passes if it is given -keysym Caps_Lock rather than -keysym Shift_Lock.

Regarding the other two failing tests I believe there is some remaining issue in the MatchPatterns function, or in some lower level function called by MatchPatterns.

Current status on Windows:

==== bind-15.23 MatchPatterns procedure, time wrap-around FAILED
==== Contents of test case:

    bind .t.f <Double-1> {set x 1}
    set x 0
    event generate .t.f <Button-1> -time -100
    event generate .t.f <Button-1> -time 500
    event generate .t.f <ButtonRelease-1>
    return $x

---- Result was:
1
---- Result should have been (exact matching):
0
==== bind-15.23 FAILED



==== bind-32.9 trigger events for modifier keys FAILED
==== Contents of test case:

    bind .t.f <Any-Key> { set x "Key" }
    event generate .t.f <KeyPress> -keysym Shift_Lock
    set x

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: no keycode for keysym "Shift_Lock"
    while executing
"event generate .t.f <KeyPress> -keysym Shift_Lock"
    ("uplevel" body line 3)
    invoked from within
"uplevel 1 $script"
---- errorCode: TK LOOKUP KEYCODE Shift_Lock
==== bind-32.9 FAILED



==== bind-32.10 clash of Space with VK_CAPITAL (Windows only) FAILED
==== Contents of test case:

    bind .t.f <1> { set x "Single" }
    bind .t.f <Double-1> { set x "Double" }
    event generate .t.f <1>
    event generate .t.f <KeyPress> -keycode 20
    event generate .t.f <1>
    set x

---- Result was:
Double
---- Result should have been (exact matching):
Single
==== bind-32.10 FAILED

Other tests from bind.test do pass in Windows.

Fossil was complaining because a fork has formed. Should be fixed now in the repository. Just do a "fossil update" and after that everything should be fine.

Now it was possible to push a new version of tkBind.c, and it seems that current version in repository is matching my working version, although Fossil still complains "multiple open leaf check-ins". So please try new version [9b04e0fe].

I have not forgotten to push. but Fossil has complained "server says: 307 Temporary Redirect", I don't know why. Now I get the messages

WARNING: multiple open leaf check-ins on bug6e8afe516d:
  (1) 2018-12-11 15:57:14 [087aca88eb] (current)
  (2) 2018-12-09 20:05:31 [a8cb724b4e]

when I try to push, what should I do, I cannot commit anymore?

Moreover Fossil says that my working version of tkBind.c is identical to the repository version, but this is wrong. I cannot understand Fossil.

It seems that I was wrong with VK_CAPITAL, but I'm not wrong that there is any clash between any modifier key and the Space, the trace is showing this. With other words, dispPtr->numModKeyCodes contains the value 0x20 (under Windoze).

I cannot find any artefact [087aca88eb] in the Tk repository. Did you forget to push perhaps?

About your questions perhaps the Windows-specific implementation in tkWinKey.c could provide you with some answers.

Regarding your claim of a Windows bug "there is a clash between the key codes of Space and VK_CAPITAL (shift lock), both have same code", I wanted to check this and used the following small script:

  package require Tk
  bind . <Key> {puts "keycode: %k state: %s keysym: %K keysym (decimal): %N Unicode char: !!%A!!"}

Then I clicked in the . window to give it the focus and pushed the space key first, and the shift lock key next. I got an output for the space key only, not for shift lock:

keycode: 32 state: 8 keysym: space keysym (decimal): 32 Unicode char: !! !!

This seems correct to me.

Moreover, Microsoft documents that the keycode for VK_CAPITAL is 0x14 whereas for VK_SPACE it is 0x20. Looks like these codes are not the same.

Many thanks for testing, with the help of the trace I could figure out the problem. Here two bugs are involved:

1. Currently the implementation is not triggering events in case of modifier keys. This is inconsistent to original implementation and has been changed in [087aca88eb].

2. But this bug has exploited a special Windoze bug, there is a clash between the key codes of Space and VK_CAPITAL (shift lock), both have same code. So it is not possible to distinguish between a Space and Shift_Lock. This means that now the following test case fails (but only under Windoze), because modifier keys should not influence multi-clicks (this is new test case 32.10):

bind $f <1> { set x "Single" }
bind $f <Double-1> { set x "Double" }
event generate $f <1>
event generate $f <KeyPress> -keycode 20
event generate $f <1>
puts $x

This script gives "Single" under Linux, but will give "Double" under Windoze (if I'm not wrong, I cannot test Windoze), because a modifier key will be wrongly detected (see line 2124 in tkBind.c).

The fix of this Windoze problem depends on the following questions:

1. Does Windoze key codes match the key codes of Linux (except modifier key codes, these codes are different)?

2. Does Windoze know key codes > 0x10000000?

3. Does Windoze already use any of these key codes?
#define XK_Shift_L 0xffe1 /* Left shift */
#define XK_Shift_R 0xffe2 /* Right shift */
#define XK_Control_L 0xffe3 /* Left control */
#define XK_Control_R 0xffe4 /* Right control */
#define XK_Caps_Lock 0xffe5 /* Caps lock */
#define XK_Shift_Lock 0xffe6 /* Shift lock */
#define XK_Meta_L 0xffe7 /* Left meta */
#define XK_Meta_R 0xffe8 /* Right meta */
#define XK_Alt_L 0xffe9 /* Left alt */
#define XK_Alt_R 0xffea /* Right alt */
#define XK_Super_L 0xffeb /* Left super */
#define XK_Super_R 0xffec /* Right super */
#define XK_Hyper_L 0xffed /* Left hyper */
#define XK_Hyper_R 0xffee /* Right hyper */

Correction, it's correct in bind-13.14: the keycode that is passed by the test is -1, which becomes 4294967295 when casted in %u for printf. No issue there.

More info. If I run the full bind.test in -verbose bepst mode, with 'fflush(stdout)' added right after your three 'printf's, I see the following output (trimmed down to those exercising the 'printf's):

[...]
---- bind-13.12 start
22.46-2: 16
22.46-2: 16
22.46-2: 16
22.46-2: 16
++++ bind-13.12 PASSED
---- bind-13.13 start
++++ bind-13.13 PASSED
---- bind-13.14 start
22.46-1: 4294967295
22.46-1: 4294967295
++++ bind-13.14 PASSED
[...]
---- bind-15.7 start
22.46-2: 16
++++ bind-15.7 PASSED
[...]
---- bind-16.16 start
22.46-1: 146
22.46-3: 1 -- 2 -- 146 -- 146
++++ bind-16.16 PASSED
[...]
---- bind-22.46 start
22.46-1: 20
22.46-2: 20

==== bind-22.46 HandleEventGenerate: options <Key> -keycode 20 FAILED
==== Contents of test case:

    bind .t.f <Key> "lappend x %k"
    event generate .t.f <Key> -keycode 20
    return $x

---- Result was:

---- Result should have been (exact matching):
20
==== bind-22.46 FAILED

At least on one occurrence (see bind-13.14) it seems something is not correctly initialized.

bind-15.23 still fails.

Regarding bind-22.46, here is the trace with my script from 30 Oct.:

22.46-1: 20
22.46-2: 20

I've committed version [618252a2d9], hopefully bind-15.23 is passing again. Please also try your short test script (provided on Oct 30th, 2018), the latest version will print some trace information, I hope that helps a bit for finding the special Windoze problem. Under Linux I see the following trace:

22.46-1: 20
22.46-3: 1 -- 2 -- 20 -- 20

For any reason a new bug has been exploited, but the latest version is indeed fixing the 64 bit problem (tested by Steve, Scid vs PC). I will look for the remaining problems, probably I will build a special test version for Windoze, because under Linux (64 bit) only one test case is failing.

The new implementation is fixing a very nasty bug, occurring in applications Scid and Scid vs PC. After switching the tab sometimes it happens that the focus is lost. With new implementation this problem is gone, tested by Steve.

Sorry, bad news.

On Windows Vista, at [18c2efa9], bind-22.46 still fails (and the corresponding short test script I provided on Oct 30th, 2018 as well). Moreover bind-15.23 is now failing as well, which was not the case before [18c2efa9].

==== bind-15.23 MatchPatterns procedure, time wrap-around FAILED
==== Contents of test case:

    bind .t.f <Double-1> {set x 1}
    set x 0
    event generate .t.f <Button-1> -time -100
    event generate .t.f <Button-1> -time 500
    event generate .t.f <ButtonRelease-1>
    return $x

---- Result was:
1
---- Result should have been (exact matching):
0
==== bind-15.23 FAILED


==== bind-22.46 HandleEventGenerate: options <Key> -keycode 20 FAILED
==== Contents of test case:

    bind .t.f <Key> "lappend x %k"
    event generate .t.f <Key> -keycode 20
    return $x

---- Result was:

---- Result should have been (exact matching):
20
==== bind-22.46 FAILED

I think I have fixed the 64 bit problem, I could find one hole after reading the sources the fifth time. Francois, please do a test.

> Race conditions? Shouldn't these be fixed (perhaps in the test)?

It would be fine to fix this, I couldn't figure out how to make these tests safe. The problem (with old and new code) is that widget .f.h might not be mapped when the event test is running. I already inserted the statement "after 250" for a little pause, which helps a bit. And trials with "tkwait visibility .f.h" failed, for any reason this hangs sometimes. Probably a loop is needed, waiting for <Map> event.

I have attached one random log when running bind.test under valgrind through "valgrind ./tktest ../tests/bind.test" under Linux Debian 8.

Race conditions? Shouldn't these be fixed (perhaps in the test)?

Many thanks for all the information.

- when running bind.test through ./tktest ../tests/bind.test there can be zero failure, but sometimes some tests fail unreliably 

Test cases 19.15, 19.16, and 19.17, may fail sometimes because of raise conditions.

- when running bind.test under valgrind through valgrind ./tktest ../tests/bind.test there are always many failures (>50, but not always the exact same number)

Please attach a log if this valgrind test. I think that the fix of this Linux problem will also fix the Windoze problem.

More info, for Linux;

- when running bind.test through make test TESTFLAGS="-file bind.test" there is zero failure

- when running bind.test through ./tktest ../tests/bind.test there can be zero failure, but sometimes some tests fail unreliably

- when running bind.test under valgrind through valgrind ./tktest ../tests/bind.test there are always many failures (>50, but not always the exact same number)

Looks like there is still some work to do to fix this.

Regarding the Windows case and bind-22.46:

The reproducible test script I provided on 2018-10-30 still allows me to reproduce the problem on Windows.

- Did you see my comment in that post on the very last line? "if in the above test script I comment "event generate .f <Control-Key-space>" then the expected "20" is obtained" Isn't this interesting?

- The test script allows to reproduce the problem if it is pasted at once in tclsh. If it is pasted line by line, then there is no problem (i.e. the expected "20" is obtained). This observation could lead to the root cause of the problem.

This brought definitely many improvements:

2) On Linux now I see zero failure in bind.test

3) On macOS also I now see zero failure in bind.test

There is a warning however:

/Users/fvogel/Documents/tcltk/fossil/tk/unix/../generic/tkBind.c:50:9: warning: 'bool' macro redefined [-Wmacro-redefined]
#define bool int
        ^
/Library/Developer/CommandLineTools/usr/lib/clang/9.0.0/include/stdbool.h:31:9: note: previous definition is here
#define bool _Bool
        ^

1) On Windows unfortunately, at [d509841817], bind-22.46 still fails identically as before.

Many thanks for the valgrind test. I've committed [d509841817], this should solve all the 64 bit problem (I hope):

1. On Windows, at [5a57749740], bind-22.46 still fails...

2. On Debian 8 Linux, I see 38 failures in bind.test...

3. On macOS it's worse. I see >70 failures with the Development target, and it hangs with the Deployment target.

If not working under Debian 8, please also do a valgrind test:

valgrind ./tktest ../tests/bind.test

On Linux, simply running wish under valgrind gives some clues I think:

francois@TM:~/Documents/tcltk/fossil/tk/unix$ valgrind /home/francois/Documents/tcltk/fossil/tcltk/bin/wish8.6
==15175== Memcheck, a memory error detector
==15175== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==15175== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==15175== Command: /home/francois/Documents/tcltk/fossil/tcltk/bin/wish8.6
==15175== 
==15175== Invalid read of size 1
==15175==    at 0x4C2D1D3: strcmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15175==    by 0x5CF5EC3: _XimUnRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x5CDC8A2: XUnregisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x4F7D87D: InstantiateIMCallback (tkUnixEvent.c:681)
==15175==    by 0x5CF5DA4: _XimRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x5CDC83B: XRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x4F7CFAB: TkpOpenDisplay (tkUnixEvent.c:184)
==15175==    by 0x4EB0DE9: GetScreen (tkWindow.c:465)
==15175==    by 0x4EB0BAA: CreateTopLevelWindow (tkWindow.c:348)
==15175==    by 0x4EB1851: TkCreateMainWindow (tkWindow.c:854)
==15175==    by 0x4EC06F9: CreateFrame (tkFrame.c:582)
==15175==    by 0x4EC0231: TkListCreateFrame (tkFrame.c:468)
==15175==  Address 0x7eae850 is 0 bytes inside a block of size 1 free'd
==15175==    at 0x4C29E90: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15175==    by 0x5CEBDFF: XSetLocaleModifiers (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x4F7D90F: OpenIM (tkUnixEvent.c:725)
==15175==    by 0x4F7D851: InstantiateIMCallback (tkUnixEvent.c:680)
==15175==    by 0x5CF5DA4: _XimRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x5CDC83B: XRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==15175==    by 0x4F7CFAB: TkpOpenDisplay (tkUnixEvent.c:184)
==15175==    by 0x4EB0DE9: GetScreen (tkWindow.c:465)
==15175==    by 0x4EB0BAA: CreateTopLevelWindow (tkWindow.c:348)
==15175==    by 0x4EB1851: TkCreateMainWindow (tkWindow.c:854)
==15175==    by 0x4EC06F9: CreateFrame (tkFrame.c:582)
==15175==    by 0x4EC0231: TkListCreateFrame (tkFrame.c:468)
==15175== 
==15175== Conditional jump or move depends on uninitialised value(s)
==15175==    at 0x4C2ED52: __memcmp_sse4_1 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15175==    by 0x4E7702B: FindSequence (tkBind.c:4513)
==15175==    by 0x4E74784: CreateVirtualEvent (tkBind.c:3342)
==15175==    by 0x4E7421F: Tk_EventObjCmd (tkBind.c:3176)
==15175==    by 0x5236103: Dispatch (tclBasic.c:4426)
==15175==    by 0x523618C: TclNRRunCallbacks (tclBasic.c:4461)
==15175==    by 0x52358FB: Tcl_EvalObjv (tclBasic.c:4189)
==15175==    by 0x5238074: TclEvalEx (tclBasic.c:5330)
==15175==    by 0x523742A: Tcl_EvalEx (tclBasic.c:4995)
==15175==    by 0x4EB51BB: Initialize (tkWindow.c:3333)
==15175==    by 0x4EB4267: Tk_Init (tkWindow.c:2914)
==15175==    by 0x400AFE: Tcl_AppInit (tkAppInit.c:109)
==15175== 
==15175== Conditional jump or move depends on uninitialised value(s)
==15175==    at 0x4E7702E: FindSequence (tkBind.c:4513)
==15175==    by 0x4E74784: CreateVirtualEvent (tkBind.c:3342)
==15175==    by 0x4E7421F: Tk_EventObjCmd (tkBind.c:3176)
==15175==    by 0x5236103: Dispatch (tclBasic.c:4426)
==15175==    by 0x523618C: TclNRRunCallbacks (tclBasic.c:4461)
==15175==    by 0x52358FB: Tcl_EvalObjv (tclBasic.c:4189)
==15175==    by 0x5238074: TclEvalEx (tclBasic.c:5330)
==15175==    by 0x523742A: Tcl_EvalEx (tclBasic.c:4995)
==15175==    by 0x538C5C9: TraceVarProc (tclTrace.c:2061)
==15175==    by 0x538D0DB: TclCallVarTraces (tclTrace.c:2730)
==15175==    by 0x538CC76: TclObjCallVarTraces (tclTrace.c:2572)
==15175==    by 0x5396FC7: TclPtrSetVarIdx (tclVar.c:1997)
==15175== 
==15175== Conditional jump or move depends on uninitialised value(s)
==15175==    at 0x4C2ED52: __memcmp_sse4_1 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15175==    by 0x4E7702B: FindSequence (tkBind.c:4513)
==15175==    by 0x4E7076C: Tk_CreateBinding (tkBind.c:1524)
==15175==    by 0x4E7C166: Tk_BindObjCmd (tkCmds.c:225)
==15175==    by 0x5236103: Dispatch (tclBasic.c:4426)
==15175==    by 0x523618C: TclNRRunCallbacks (tclBasic.c:4461)
==15175==    by 0x52358FB: Tcl_EvalObjv (tclBasic.c:4189)
==15175==    by 0x5238074: TclEvalEx (tclBasic.c:5330)
==15175==    by 0x523742A: Tcl_EvalEx (tclBasic.c:4995)
==15175==    by 0x4EB51BB: Initialize (tkWindow.c:3333)
==15175==    by 0x4EB4267: Tk_Init (tkWindow.c:2914)
==15175==    by 0x400AFE: Tcl_AppInit (tkAppInit.c:109)
==15175== 
==15175== Conditional jump or move depends on uninitialised value(s)
==15175==    at 0x4E7702E: FindSequence (tkBind.c:4513)
==15175==    by 0x4E7076C: Tk_CreateBinding (tkBind.c:1524)
==15175==    by 0x4E7C166: Tk_BindObjCmd (tkCmds.c:225)
==15175==    by 0x5236103: Dispatch (tclBasic.c:4426)
==15175==    by 0x523618C: TclNRRunCallbacks (tclBasic.c:4461)
==15175==    by 0x52358FB: Tcl_EvalObjv (tclBasic.c:4189)
==15175==    by 0x5238074: TclEvalEx (tclBasic.c:5330)
==15175==    by 0x523742A: Tcl_EvalEx (tclBasic.c:4995)
==15175==    by 0x4EB51BB: Initialize (tkWindow.c:3333)
==15175==    by 0x4EB4267: Tk_Init (tkWindow.c:2914)
==15175==    by 0x400AFE: Tcl_AppInit (tkAppInit.c:109)
==15175==    by 0x4E9DADE: Tk_MainEx (tkMain.c:333)
==15175== 
==15175== Conditional jump or move depends on uninitialised value(s)
==15175==    at 0x4C2ED52: __memcmp_sse4_1 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15175==    by 0x4E7702B: FindSequence (tkBind.c:4513)
==15175==    by 0x4E70C71: Tk_GetBinding (tkBind.c:1680)
==15175==    by 0x4E7C1CA: Tk_BindObjCmd (tkCmds.c:233)
==15175==    by 0x5236103: Dispatch (tclBasic.c:4426)
==15175==    by 0x523618C: TclNRRunCallbacks (tclBasic.c:4461)
==15175==    by 0x52358FB: Tcl_EvalObjv (tclBasic.c:4189)
==15175==    by 0x5238074: TclEvalEx (tclBasic.c:5330)
==15175==    by 0x523742A: Tcl_EvalEx (tclBasic.c:4995)
==15175==    by 0x4EB51BB: Initialize (tkWindow.c:3333)
==15175==    by 0x4EB4267: Tk_Init (tkWindow.c:2914)
==15175==    by 0x400AFE: Tcl_AppInit (tkAppInit.c:109)
==15175== 
==15175== Conditional jump or move depends on uninitialised value(s)
==15175==    at 0x4E7702E: FindSequence (tkBind.c:4513)
==15175==    by 0x4E70C71: Tk_GetBinding (tkBind.c:1680)
==15175==    by 0x4E7C1CA: Tk_BindObjCmd (tkCmds.c:233)
==15175==    by 0x5236103: Dispatch (tclBasic.c:4426)
==15175==    by 0x523618C: TclNRRunCallbacks (tclBasic.c:4461)
==15175==    by 0x52358FB: Tcl_EvalObjv (tclBasic.c:4189)
==15175==    by 0x5238074: TclEvalEx (tclBasic.c:5330)
==15175==    by 0x523742A: Tcl_EvalEx (tclBasic.c:4995)
==15175==    by 0x4EB51BB: Initialize (tkWindow.c:3333)
==15175==    by 0x4EB4267: Tk_Init (tkWindow.c:2914)
==15175==    by 0x400AFE: Tcl_AppInit (tkAppInit.c:109)
==15175==    by 0x4E9DADE: Tk_MainEx (tkMain.c:333)
==15175== 
% 
% exit
==15175== 
==15175== HEAP SUMMARY:
==15175==     in use at exit: 3,212,226 bytes in 1,001 blocks
==15175==   total heap usage: 37,422 allocs, 36,421 frees, 14,793,182 bytes allocated
==15175== 
==15175== LEAK SUMMARY:
==15175==    definitely lost: 408 bytes in 1 blocks
==15175==    indirectly lost: 0 bytes in 0 blocks
==15175==      possibly lost: 2,365,440 bytes in 139 blocks
==15175==    still reachable: 846,378 bytes in 861 blocks
==15175==         suppressed: 0 bytes in 0 blocks
==15175== Rerun with --leak-check=full to see details of leaked memory
==15175== 
==15175== For counts of detected and suppressed errors, rerun with: -v
==15175== Use --track-origins=yes to see where uninitialised values come from
==15175== ERROR SUMMARY: 633 errors from 7 contexts (suppressed: 0 from 0)
francois@TM:~/Documents/tcltk/fossil/tk/unix$ 

The first report with XimUnRegisterIMInstantiateCallback is known, see [e42eef33ee], and it also happens in core-8-6-branch.

The other reports state "Conditional jump or move depends on uninitialised value(s)", the problem being in FindSequence (tkBind.c:4513). This does not happen in core-8-6-branch.

> On Debian 8 Linux, I see 38 failures in bind.test.

> Using a debugger is not easy because when I create a script with just the code from bind-22.46, then this code does not fail (the expected "20" is obtained).

This looks like a memory problem. Is it possible for you to use valgrind when running bind.test? If not, then it might help to use valgrind under Linux, because I think this is another 64 bit related problem. Under 32 bit the code is clean.

About 1):

A more minimal test script showing the problem is:

    package require Tk
    frame .f
    pack .f
    focus -force .f
    update
    set x {}
    event generate .f <Control-Key-space>
    bind .f <Key> "lappend x %k"
    focus -force.f ; event generate .f <Key> -keycode 20
    return $x
    # expected: 20  -->  wrongly returns ""

It seems that I was wrong in my previous answer: all breakpoints are hit. I must have tested without ensuring the window had focus. Starting over, here are the first debugging steps:

- HandleEventGenerate seems OK as already said. Line 3912 has number = 20. Then it calls Tk_HandleEvent.

- Tk_HandleEvent calls TkBindEventProc and then Tk_BindEvent.

- line 2199: eventPtr->xkey.keycode contains 20?

--> YES

- line 2258: psl[1] not NULL after executing this statement?

--> YES

- line 2263: After executing this statement 'psl[1]->first->psPtr' should contain: {numPats=1, count=1, ..., script="lappend x %k", object=".t.f", ..., ptr={..., pats={{eventType=2, modStateMask=0, count=1, info=0, name=0x0}}}

--> YES, EXCEPT that object is {refCount=26158 bytes=0x726f74ef <Bad Ptr> length=-280034833 ...}

Note however that if in the above test script I comment "event generate .f <Control-Key-space>" then the expected "20" is obtained, despite the "object" field above still does not contain your expected value.

1)

Using a debugger is not easy because when I create a script with just the code from bind-22.46, then this code does not fail (the expected "20" is obtained).

And indeed bind-22.46 does not fail when run in isolation. Narrowing down:

Fails:
  - TESTFLAGS="-file bind.test"
  - TESTFLAGS="-file bind.test -match 22.*"
  - TESTFLAGS="-file bind.test -match 22.[14]?"
  - TESTFLAGS="-file bind.test -match 22.[14][06]"

Does not fail:
  - TESTFLAGS="-file bind.test -match 22.46"
  - TESTFLAGS="-file bind.test -match 22.4?"
  - TESTFLAGS="-file bind.test -match 22.[234]?"
  - TESTFLAGS="-file bind.test -match 22.[14]6"

After some more time narrowing down, it appears that bind-22.46 fails if bind-22.10 was run before it. Therefore the folowing resulting small script reproducing the failure of bind-22.46 (I didn't try to make this minimal):

package require Tk

toplevel .t

    frame .t.f -class Test -width 150 -height 100
    pack .t.f
    focus -force .t.f
    update
    set x {}

    bind .t.f <Key> {set x "%s %K"}
    event generate .t.f <Control-Key-space>
    set x
# expected: {4 space}  -->  OK

    destroy .t.f


    frame .t.f -class Test -width 150 -height 100
    pack .t.f
    focus -force .t.f
    update
    set x {}

    bind .t.f <Key> "lappend x %k"
    event generate .t.f <Key> -keycode 20
    return $x
# expected: 20  -->  wrongly returns ""

I can now answer your questions:

line 3912: event.general.xkey.keycode = number; // number == 20 ?

--> Yes.

The other breakpoints you proposed are not hit when running the "event generate" codeline. I didn't study your code so far to understand why.

2+3+4) OK, later

5) Thanks!

1) On Windows, at [5a57749740], bind-22.46 still fails identically as before.

   Mysterious for me. Is it possible for you to use a debugger:

   line 3912: event.general.xkey.keycode = number; // number == 20 ?

   line 2199: eventPtr->xkey.keycode contains 20?

   line 2258: psl[1] not NULL after executing this statement?

   line 2263: After executing this statement 'psl[1]->first->psPtr' should contain: {numPats=1, count=1, ..., script="lappend x %k", object=".t.f", ..., ptr={..., pats={{eventType=2, modStateMask=0, count=1, info=0, name=0x0}}}

2+3+4) Later.

5) I've added a commentary to [796462af24].

Report:

1. On Windows, at [5a57749740], bind-22.46 still fails identically as before.

2. On Debian 8 Linux, I see 38 failures in bind.test. I'm attaching the corresponding failure log "Debian8_bindtest_5a577497.txt".

3. On macOS it's worse. I see >70 failures with the Development target, and it hangs with the Deployment target. Let me know if you'd like to see a log.

4. Regarding your question below, I think we should fix the problem and return "Double". It's a very minor inconsistency and I agree nobody is using <1><1> (never seen this in any code). What we could do is we wrap this around another PREFER_MOST_SPECIALIZED_EVENT case, we don't change the behavior in the bugfix branch bug6e8afe516d, nor in core-8-6-branch when it will be merged, but when merging in trunk we define that constant (and also SUPPORT_ADDITIONAL_MOTION_SYNTAX, btw). Perhaps we should also improve the documentation on these two points.

5. Did you see Marc's analysis in [796462af24]? Is your new implementation taking full advantage of the extra event information available on a mac (I mean, the 'clickCount' field)?

One more bug (overtaken from legacy code) still exists:

bind .t.f <1><1> { lappend x "11" }
bind .t.f <Double-1> { lappend x "Double" }
event generate .t.f <1> -x 0 -time 0
event generate .t.f <1> -x 0 -time 1000

This gives the correct result 11.

bind .t.f <Double-1> { lappend x "Double" }
bind .t.f <1><1> { lappend x "11" }
event generate .t.f <1> -x 0 -time 0
event generate .t.f <1> -x 0 -time 0

This gives the result 11, but Double is expected, because the time (and space) constraints for a double click are fulfilled.

If we fix this then we are not fully backwards compatible anymore. But in my opinion this is not a problem, nobody is using the sequence <1><1>. (And furthermore such incompatibilities are a Windoze only problem, under Unix/Mac the applications are bound to the appropriate library versions.) What should we do?

I've committed [fe6bb55251], from the algorithmic point of view this should be the final version. Except bug fixing of course, it seems that one or two technical problems still exist, a friend is testing under Mint.

Please Francois, do a Windoze test. I hope that the problem with bind-22.46 has gone.

I've committed [c6dc1fea9d], this should finally solve the 64 bit problem (uninitialized bytes in hash table key).

I don't have another test script. I've committed [ad48b5a072] fixing a bug in array implementation, I hope that is solving the problem.

Success! It compiles on Windows (Vista 64 bits) flawlessly.

Running the bind.test suite, the only failing test is:

==== bind-22.46 HandleEventGenerate: options <Key> -keycode 20 FAILED
==== Contents of test case:

    bind .t.f <Key> "lappend x %k"
    event generate .t.f <Key> -keycode 20
    return $x

---- Result was:

---- Result should have been (exact matching):
20
==== bind-22.46 FAILED

Regarding what is said to not work:

  package require Tk
  bind . <1> {puts hello}
  # click in the . window

This prints "hello" as expected on my Win Vista 64 bits. Do you have a different test script that would show the problem?

I've committed [ea08013f29], hopefully Windoze will compile.

About the 64 bit problem: currently the only thing I know is that event binding (like "bind . <1> {...}") is not working for any reason, still looking for the problem.

The best way to test is
    make tktest
    ./tktest ../tests/bind.test
And it is recommended to compile with debug info, and without -DNDEBUG=1, because the assertions should be enabled.

Many thanks for testing, and for appreciating.

I will look for those compile problems under Windows. Unfortunately there seems to be any problem, probably a 64 bit related thing, because on my old machine it works perfectly, but on other machines - and most people have 64 bit systems - it hangs. A friend is currently testing, I hope he will detect the reason.

This looks like a _very_ nice contribution, once more, many thanks!

I tried compilation with MSVC but got errors:

tkBind.c
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(175) : error C2220: warning treated as error - no 'object' file generated
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(175) : warning C4018: '<=' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(175) : warning C4018: '<=' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(175) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(175) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(259) : warning C4018: '<=' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(259) : warning C4018: '<=' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(259) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(259) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(343) : warning C4018: '<=' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(343) : warning C4018: '<=' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(343) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(343) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1087) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1121) : error C2143: syntax error : missing ';' before 'type'
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1123) : error C2275: 'BindInfo' : illegal use of this type as an expression
        C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(372) : see declaration of 'BindInfo'
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1123) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1199) : error C2065: 'initialized' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1201) : error C2065: 'initialized' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1245) : error C2065: 'initialized' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1252) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1252) : warning C4047: '=' : 'int' differs in levels of indirection from 'void *'
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1253) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1253) : error C2223: left of '->virtualEventTable' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1253) : error C2198: 'InitVirtualEventTable' : too few arguments for call
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1254) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1254) : error C2223: left of '->screenInfo' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1255) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1255) : error C2223: left of '->screenInfo' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1256) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1256) : error C2223: left of '->screenInfo' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1257) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1257) : error C2223: left of '->deleted' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1258) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1258) : error C2223: left of '->lastCurrentTime' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1259) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1259) : error C2223: left of '->lastEventTime' must point to struct/union
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1260) : error C2065: 'bindInfoPtr' : undeclared identifier
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1260) : warning C4047: '=' : 'TkBindInfo' differs in levels of indirection from 'int'
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1512) : warning C4018: '>' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(1787) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(2055) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(2182) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(2214) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(2510) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(3475) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(3689) : warning C4018: '<' : signed/unsigned mismatch
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkBind.c(4509) : error C2065: 'copy' : undeclared identifier

I've checked in 67dd75c73160be69.

A simple fix wasn't impossible, because the idea with the event ring is not working, the overflow problem of this ring is not solvable. For example bug (1), in this case the problem is that a couple of events may intervene, and the overflow of the event ring disallows the detection of a double-click, only a single-click will be triggered in such cases. Also bug (2) is a reproducible overflow problem. In new implementation I've eliminated the event ring, the new idea is to work with promoted event sequences. This idea is eliminating any problem with overflows, as if we have an infinite event ring. All these bugs now are fixed:

1) No more overflow problems, every double-click will be detected, tested with application Scidb; see also test case bind-32.2.

2) No more overflow problems (causing that the modifier state will not be detected properly), opening the menu immediately after startup works, tested with application Scidb.

3) It's only a minor bug, and there exists a work-around. But I decided to eliminate this design bug, with new implementation option "-time" is recognizing the special value "current", and is using the current event time in this case. This extension is fully backward compatible. See also test case bind-32.4.

4) Old implementation is computing the time difference of nth click with fst click, and tests whether it is less than 500 ms. But this seems to be an implementation bug. New implementation computes the difference of nth and (n+1)th click. This behavior also conforms better to the behavior of other toolkits. With new implementation the use of quadruple clicks is unproblematic. See also test case bind-32.5.

5) Fixed, see test case bind-32.6. For the fix I decided that every non-zero value (given with option -send_event) will be converted to 1 (test case bind-22.92 has been changed accordingly). This is conform to the manual, see also "man bind" (search for "Sendevent"), and see lines 3280 ff in old file tkBind.c.

Some more implementation bugs are fixed:

a) DeleteVirtualEventTable() did not free 'script' (memory hole).

b) 'voPtr' will not be freed at several places in old implementation (memory holes).

c) Old concept with 'nearby' flag in 'struct PatSeq' cannot work, example:
    <1><Double-1><1>
'nearby' should not have any impact on <1>, but it has. New implementation doesn't need this flag.

One thing has been changed, see test case bind-27.2. The statement "bind . <Button-6> {}" now will trigger error
    bad button number "6"
instead of
    specified keysym "6" for non-key event
this is more logical, and I think that old implementation was erroneous. This change is the result of an overwork of function ParseEventDescription(), I couldn't understand the logic in old implementation, the new implementation is easier to understand.

Old implementation is handling <Double-1> and <1><1> as equivalant. IMO this handling is quite unluckily, <Double-1> should have higher precedence (see also new test case bind-33.2), because it has to fit time and space requirements. But I have not touched this behavior, although I think that a change of this behavior would be unproblematic. And I already prepared the implementation for a change of this handling, see constant PREFER_MOST_SPECIALIZED_EVENT.

The new implementation is passing all test cases, and works properly with application Scidb. Based on my test the performance in time is better than with old implementation. This result is expected, because a triple-nested loop has been changed to a quasi-double-nested loop (only in very seldom cases it is still triple-nested). Furthermore the traversed lists are shorter than with old implementation, because the event ring, always containing 30 items, has been eliminated. Only unbinding a tag is a bit slower than before. Memory consumption did not change significantly.

The following problems are still unsolved:

1) TkpSetKeycodeAndState() is generating keyocde '$' for key symbol 'Return'. This is obviously wrong, function TkpSetKeycodeAndState() seems to have any problem.

2) Sometimes <ButtonRelease> event will not be triggered (with old and new implementation). This problem is very nasty, and not reproducible. But I'm sure that this problem occurs at another place, not in tkBind.c.

3) Quite often event <Unmap> will not be triggered (with old and new implementation). This is a severe bug, and not reproducible. Here I'm also sure that this problem occurs at another place, not in tkBind.c.

4) After startup of an application opening a menu with right-click will close the menu immediately (woth old and new implementation). Only after second right-click the menu will be kept open. I'm still searching for the reason of this bug. Here I'm also sure that this is not a problem within tkBind.c.

Suggestion:

Due to documentation it's possible to combine motion events with buttons, for example <B2-Motion>. For any reason the syntax <Motion-2> is not allowed, but I think it makes sense to allow the latter syntax. And in this way it should be allowed to express <B1-B2-Motion> with <Motion-1-2>. I've already implemented this suggestion, but it is commented out (not yet active, see compiler constant SUPPORT_ADDITIONAL_MOTION_SYNTAX).