Fix in comparison of complex patterns, new test cases bind-33.9, bind-33.10, and bind-33.11.

2 files changed, 84 insertions, 8 deletions

diff --git a/generic/tkBind.c b/generic/tkBind.c
index aff6418..43e4655 100644
--- a/generic/tkBind.c
+++ b/generic/tkBind.c
@@ -719,6 +719,10 @@ static void		RemovePatSeqFromLookup(LookupTables *lookupTables, PatSeq *psPtr);
 static void		RemovePatSeqFromPromotionLists(Tk_BindingTable bindPtr, PatSeq *psPtr);
 static PatSeq *		DeletePatSeq(PatSeq *psPtr);
 static void		InsertPatSeq(LookupTables *lookupTables, PatSeq *psPtr);
+#if SUPPORT_DEBUGGING
+void			TkpDumpPS(const PatSeq *psPtr);
+void			TkpDumpPSList(const PSList *psList);
+#endif
 
 /*
  * Some useful helper functions.
@@ -2044,6 +2048,9 @@ IsBetterMatch(
     }
 
 #if PREFER_MOST_SPECIALIZED_EVENT
+    if (fstMatchPtr->numPats > sndMatchPtr->numPats) { return true; }
+    if (fstMatchPtr->numPats < sndMatchPtr->numPats) { return false; }
+
     for (i = patIndex; i >= 0; --i) {
 	unsigned fstCount = GetCount(fstMatchPtr, i);
 	unsigned sndCount = GetCount(sndMatchPtr, i);
@@ -2056,11 +2063,6 @@ IsBetterMatch(
     if (sndMatchPtr->count > fstMatchPtr->count) { return true; }
     if (sndMatchPtr->count < fstMatchPtr->count) { return false; }
 
-#if PREFER_MOST_SPECIALIZED_EVENT
-    if (fstMatchPtr->numPats > sndMatchPtr->numPats) { return true; }
-    if (fstMatchPtr->numPats < sndMatchPtr->numPats) { return false; }
-#endif
-
     return sndMatchPtr->number > fstMatchPtr->number;
 }
 
@@ -2637,6 +2639,9 @@ Compare(
 
     if (!fstMatchPtr) { return +1; }
 
+    assert(patIndex < fstMatchPtr->numPats);
+    assert(patIndex < sndMatchPtr->numPats);
+
     for (i = patIndex; i >= 0; --i) {
 	unsigned long fstInfo = GetInfo(fstMatchPtr, i);
 	unsigned long sndInfo = GetInfo(sndMatchPtr, i);
@@ -2645,6 +2650,10 @@ Compare(
 	if (!sndInfo && fstInfo) { return -1; }
     }
 
+    if (patIndex == fstMatchPtr->numPats - 1 && patIndex == sndMatchPtr->numPats - 1) {
+	return (int) sndMatchPtr->count - (int) fstMatchPtr->count;
+    }
+
     return (int) GetCount(sndMatchPtr, patIndex) - (int) GetCount(fstMatchPtr, patIndex);
 }
 
@@ -2725,11 +2734,19 @@ MatchPatterns(
 				 * This is also a final pattern.
 				 * We always prefer the pattern with better match.
 				 *
-				 * In case of equality: choose this match if modifier states is
-				 * not a subset of those in best match.
+				 * In case of equal modifier masks:
+				 *     this match has same mask as best match:
+				 *         choose this match if number is higher
+				 *     otherwise:
+				 *         choose this match if modifier states is not a subset
+				 *         of those in best match.
 				 */
 
-				if (cmp > 0 || (cmp == 0 && !IsSubsetOf(modMask, bestModMask))) {
+				if (cmp > 0
+				    || (cmp == 0
+					&& (modMask == bestModMask
+					    ? bestPtr->number < psPtr->number
+					    : !IsSubsetOf(modMask, bestModMask)))) {
 				    bestPtr = psPtr;
 				    bestModMask = modMask;
 				    if (physPtrPtr) {
diff --git a/tests/bind.test b/tests/bind.test
index e3dbae9..2d46f9b 100644
--- a/tests/bind.test
+++ b/tests/bind.test
@@ -6474,6 +6474,65 @@ test bind-33.8 {prefer most specialized} -setup {
 } -cleanup {
     destroy .t.f
 } -result {1}
+test bind-33.9 {prefer last in case of homogeneous patterns} -setup {
+    pack [frame .t.f]
+    focus -force .t.f
+    update
+    set x {}
+} -body {
+    bind .t.f <1><2><2><Double-1> { lappend x "first" }
+    bind .t.f <1><Double-2><1><1> { lappend x "last" }
+    event generate .t.f <1>
+    event generate .t.f <2>
+    event generate .t.f <2>
+    event generate .t.f <1>
+    event generate .t.f <1>
+    set x
+} -cleanup {
+    destroy .t.f
+} -result {last}
+test bind-33.10 {prefer last in case of homogeneous patterns} -setup {
+    pack [frame .t.f]
+    focus -force .t.f
+    update
+    set x {}
+} -body {
+    bind .t.f <1><Double-2><1><1> { lappend x "first" }
+    bind .t.f <1><2><2><Double-1> { lappend x "last" }
+    event generate .t.f <1>
+    event generate .t.f <2>
+    event generate .t.f <2>
+    event generate .t.f <1>
+    event generate .t.f <1>
+    set x
+} -cleanup {
+    destroy .t.f
+} -result {last}
+test bind-33.11 {should prefer most specialized} -setup {
+    pack [frame .t.f]
+    focus -force .t.f
+    update
+    set x {}
+} -body {
+    bind .t.f <2><Double-1><Double-2><Double-1><2><2> { lappend x "first" }
+    bind .t.f <2><1><1><2><2><Double-1><Double-2> { lappend x "last" }
+    event generate .t.f <2>
+    event generate .t.f <1>
+    event generate .t.f <1>
+    event generate .t.f <2>
+    event generate .t.f <2>
+    event generate .t.f <1>
+    event generate .t.f <1>
+    event generate .t.f <2>
+    event generate .t.f <2>
+    set x
+} -cleanup {
+    # This test case shows that old implementation has an issue, because
+    # in my opinion it is expected that first one is matching, this binding
+    # is more specialized. But new implementation will be conform to old,
+    # and so "last" is the expected result.
+    destroy .t.f
+} -result {last}
 
 
 # cleanup