Tk Source Code

A. Problem description
======================
If a binding script is registered for a specific event on a specific
binding tag, Tk normally invokes that script when it receives notification
of the event from the windowing system (or from Tk itself in case of win32).
There is one single, remarkable exception: Tk skips the invocation of binding
scripts for crossing events* and focus events** if the detail field for
the event is "NotifyInferior".

		*  <Leave> and <Enter>
		** <FocusOut> and <FocusIn>

The code section where this happens is:

    [https://core.tcl-lang.org/tk/file?udc=1&ln=on&ci=a44363e604dadc30&name=generic%2FtkBind.c&ln=2209,2236]

This behaviour isn't described in user-documentation, but a comment in the
above code section reveals the reason for doing so:

....
    /*
     * Ignore the event completely if it is an Enter, Leave, FocusIn, or
     * FocusOut event with detail NotifyInferior. The reason for ignoring
     * these events is that we don't want transitions between a window and its
     * children to be visible to bindings on the parent: this would cause
     * problems for mega-widgets, since the internal structure of a
     * mega-widget isn't supposed to be visible to people watching the parent.
     *
     * ...
     */
....


B. Critique
===========
First off: the argumentation in this section focuses on crossing events
because that's what I am most familiar with. But I'm not aware of any
reasons why these arguments aren't applicable to focus events.

Ignoring binding scripts for crossing events with detail field NotifyInferior
is questionable because:

  1. It's at the expense of regular widgets
  -----------------------------------------
  The code comment in section A. mentions problems for mega-widgets. However,
  as implemented, the choice to ignore these binding scripts applies to all
  windows in a parent-child relationship, not just mega-widgets.
  Therefore, all windows in a Tk application are affected (except in the
  extreme case where an application consists of just the root window).

  A similar indiscriminate approach holds for the test in Tk test suite
  that exercises the chosen behaviour (this is test bind-13.10 in file
  bind.test). This test uses a regular parent and child window, so
  there is nothing particularly specific for mega-widgets here.

  Because of the indiscriminate nature of the current practice, Tk
  effectively deprives its regular widgets of events.

  2. (under reserve) It's ineffective
  -----------------------------------
  The code comment in section A. above mentions the purpose of the
  current practice: "keeping the internal structure of mega-widgets
  invisible for people watching the parent".

  Reservation:  If this is meant as "invisible for people watching the
  parent through bindings to crossing events between child and parent",
  I can see the logic, and this second argument may be moot. Otherwise:

  The "attack surface" for someone determined to reveal the internal
  structure of a parent window***, is much wider than just crossing
  events with detail field NotifyInferior.
  
		*** this includes mega-widgets
  
  For example:
  a. a plain [winfo children $parent], combined with a [winfo geometry $child]
     for each child window, is the canonical way for revealing the internal
     structure of the parent (and very powerful at that).
  b. crossing events with detail field NotifyAncestor. When moving the mouse
     pointer between two windows in a parent-child relationship, two crossing
     events are generated: one with detail field NotifyInferior (tartgeted at
     one widget) and one with detail field NotifyAncestor (targeted at the
     other widget). In other words: these crossing events come in pairs. Tk
     currently hides only one of them, as if the other half of the pair
     doesn't reveal anything.
  c. crossing events with detail NotifyNonlinear. Like case b., scripts
     bound to crossing events are not ignored in this case. These events
     occur when the mouse pointer transitions directly between the child
     window of the mega-widget and another toplevel window or any of its
     children (you would need to position the other toplevel window partly
     over the mega-widget's child window to be able to obtain the effect).
  d. scripts bound to screen drawing events like <Map>, <Unmap>, <Configure>,
     <Visibility> and <Expose>, also allow to witness the internal structure.
     As opposed to the previous two items, no requirements to window ancestry
     apply: binding scripts are triggered for parent windows as well as child
     windows.
  e. external programs that operate at the level of the windowing system, thus
     bypassing the Tcl/Tk process (e.g. xwininfo for X11, and WinSpy++ for
     MS Windows).

Crude summary: the current practice is a poorly targeted measure. It is
indiscriminate with respect to the windows that it affects and hence
deprives regular widgets from events. Depending on the true purpose of
Tk's choice, its effectiveness isn't great.


C. Other tickets affected by this issue
=======================================
The issue affects the following other tickets, either as the true cause of
the there observed misbehaviour, or as an additionally interfering
circumstance: [f2964ba8dd], [b049d651cf] and [9e1312f32c]. These tickets
have in common that they report missing crossing events under the specific
circumstances in those tickets.

Those dependent tickets have been around a while without making progress,
or without having addressed the true cause of the issue. Section E. provides
recommendations for the dependent tickets.


D. Patch
========
Applying the attached patch "tk.patch", will undo the skipping of binding
scripts for events with detail field NotifyInferior. This holds for crossing
events as well as focus events. It also adjusts test bind-13.10.
Additionally, it removes test event-9 because that test is rendered invalid
if bindings scripts with detail field NotifyInferior are being invoked.
The patch is based on Tk release 8.6.13.


E. Handling dependent tickets
=============================
The dependent tickets [f2964ba8dd], [b049d651cf] and [9e1312f32c] will receive
an informational follow-up comment that:
- points out any misunderstandings regarding the workings of the reported
  behaviour
- explains how Tk's deliberate choice leads to, or contributes to the
  misbehaviour reported in that ticket.
- (if practical) provides evidence of the role of the current issue by showing
  the result of the reproducible scripts in those tickets when run with a Tk
  version that has the patch under D. applied.

Erik Leunissen.

Thanks for your feedback. Complementary fix now merged. Closing.

We're done as far as I'm concerned.
Thanks for your guidance.

Right. Now fixed, thanks.

The CI runners are happy. Any objection wrt closure of this ticket on your side?

There is a copy/paste error in the comment at:

 [https://core.tcl-lang.org/tk/artifact?name=28c1d18d8cac09cb&ln=1815]

   "canvas items" should be changed to "tags"

Regarding

> I didn't check whether my explanation for ButtonEventProc does apply or not
> to all 25 cases, but it looks like you did since your NotifyInferior-V3.ods 
> settles all these cases. If that's really the case, then everything is fine
> for me. 

Only for tkFocus.c the purpose differs from your explanation.
But for me it was clear that this code is unrelated to the decision to invoke binding scripts or not.

Again with the correct commit ids:

I agree with and have committed your patch with comment changes, see [c26fb4a3].

Aside of this, regarding the panedwindow <<EnterChild>> event and the <<NOTE-PW-LEAVE-NOTIFYINFERIOR>> thing, I have implemented the changes I had described in NotifyInferior-V2.ods. Commit [927b68cf] should ease your understanding of what I had described.

I didn't check whether my explanation for ButtonEventProc does apply or not to all 25 cases, but it looks like you did since your NotifyInferior-V3.ods settles all these cases. If that's really the case, then everything is fine for me.

I agree with and have committed your patch with comment changes, see [927b68cf].

Aside of this, regarding the panedwindow <<EnterChild>> event and the <<NOTE-PW-LEAVE-NOTIFYINFERIOR>> thing, I have implemented the changes I had described in NotifyInferior-V2.ods. Commit [927b68cf] should ease your understanding of what I had described.

I didn't check whether my explanation for ButtonEventProc does apply or not to all 25 cases, but it looks like you did since your NotifyInferior-V3.ods settles all these cases. If that's really the case, then everything is fine for me.

Please find attached file tk-2.patch, which implements the changes for the
remaining cases in the rest of the Tk code base, based on the updated spreadsheet
NotifyInferior-V3.ods (also attached). All of them are changes to comments,
so there are no changes to behaviour.

The patch doesn't make a change to the case of ttk_panedWindow, because
I'm still unsure about what you suggested (and maybe we didn't understand each other well) for the following reason:

The comment in lines 469-480 explains the <<EnteredChild>> virtual event
as a replacement for a <Leave> event with detail NotifyInferior. So, it
relies on binding scripts with detail field NotifyInferior not being invoked.

I wouldn't be surprised that if we undo the skipping of binding scripts
for events with detail NotifyInferior, and also keep the binding of the
panedWindow to <<EnteredChild>>, then we'll end up invoking two binding
scripts where only one is needed (when the mouse pointer enters the parent
window, coming from the child). This would imply that
library/ttk.panedwindow.tcl:26 needs adjustment too.

Thanks for the review. I'm going to create a patch for those cases/lines for which the need of a change remains.

Please find attached my analysis in a V2 version of your spreadsheet, namely "NotifyInferior-V2.ods".

I have checked all cases/lines and have written my changes and comments in blue text color in the file. Cases are:

  1.  The easy or even obvious ones: simply noted "Agreed".

  2.  The <<EventChild>> event for Ttk panedwindow. Here I disagree with your proposed course of action. This concerns several lines of the analysis file.

  3.  The PickCurrentItem() (canvas code) and TkTextPickCurrent() (test tag code), where we could discuss if we improve the code or not. I suggest we shouldn't, and explain why.

  4.  All the 25 cases in the xxxEventProc() functions. I have analyzed in depth the ButtonEventProc() case and I can explain what this code is doing and why we should not change it. I have only written an explanation for ButtonEventProc() but I expect the reasoning to apply to all 25 xxxEventProc cases.

Hi Francois,

I've duplicated your search for NotifyInferior in the Tk code base. To provide
overview, I've listed all cases in the attached spreadsheet NotifyInferior.ods
(OpenOffice/LibreOffice format).

I've been trying for each case to decide what to do with it, based on an
answer to the following question:

  Is the identifier "NotifyInferior" used for the same purpose as what we
  intend to remove with the present ticket, or is it a consequence/corollary
  of Tk's choice that we intend to remove with the present ticket?

If the answer to the question is YES, a corrective action is needed.

An indication, but no proof, for YES is when the code line selects or
deselects "NotifyInferior" as the sole exception for the event's detail
field, for example:

    if (eventPtr->xfocus.detail != NotifyInferior) {

I've recorded all my findings in the spreadsheet, including a suggestion
for a corrective action.

There are some 25 cases that are of the same kind, and where I simply cannot decide.
This is where I believe that my understanding of the reason for the code line
is insufficient.

All of them are code lines in xxxEventProc, or the corresponding tests, where
xxx is a widget type. The code lines do the same thing across all these widgets.
Therefore, if we figure out the purpose of this kind of line, then we resolve
a whole bunch of cases in one swipe. I've marked all cases where I'm unsure as
"unsure" in the spreadsheet.

I hope that you're willing to have a look at them, and anything highlighted in a
reddish colour.

Well found! This needs to be investigated indeed.

I'm sorry that I overlooked these usages of NotifyInferior for my initial post and patch.

For now, I'm going to read the code sections you indicated, to judge from the code comments what's relevant for this ticket. In-depth investigation with code changes needs to wait several days (as far as I'm concerned).

Hmmm, not sure we're done with this one.

Searching for "NotifyInferior" in the entire Tk code brings up several places where Tk takes measures that deal with the fact events with NotifyInferior detail are ignored. This case is no longer ignored but the rest of the code seems in need of an update accordingly.

Examples of this are in all the xxxEventProc such as for instance ButtonEventProc, the canvas code, the TkFocusFilterEvent function, the TkPointerEvent function, the TkTextPickCurrent function, and several places in Ttk that should be checked carefully. I'm reopening this ticket until these are solved.

Final note: I have found the origin of the NotifyInferior filtering here. So that was in 1995, in anticipation of megawidgets.

OK, merged in core-8-6-branch and trunk.

Please sea [22349fc78a] for a reason to merge into core-8.6 branch after all.

Thanks Csaba!

I have now [a0785092|rebased the patch to trunk]. If Github Actions does not reveal issues I'll merge tomorrow.

I have tested the patch with the mega-widgets from the Tablelist, Mentry, and Scrollutil packages. AFAICT, it doesn't cause any problems, everything seems to work as before.

Yes, I am all for merging the patch without further ado.

I understand and agree with your reasons for merging into 9.0 only.

I believe that the information 1.-6. you mention is not available, to my knowledge at least. Well, I have my own ideas about some of the answers but I definitely don't want to start an in-depth lengthy discussion about this.

My understanding is we are aligned in the fact we should merge the patch, for the reasons I stated in my previous message, and also for the reasons you state in your last message below (with which I agree).

The last thing I'm wondering about is whether we can merge the patch in 8.6 and upwards, or if it should be merged in 9.0 only. If there is a risk of breaking something in 8.6, let's merge in 9.0 only. Currently I'm not 100% sure we won't break megawidgets in some (quite small, but nevertheless) extent. I'm inclined to merge in 9.0 only. If someone can provide the missing answers and wants to backport to 8.6, this can always be done.

My thoughts and point of view about this:

The possibly remaining issue that you mention, seems to be that there is no solid, useable, concrete, authoritative, documented info about:

1. what is the essential difference between  a mega-widget and a regular widget in terms of their behaviour?
2. what (preferably simple) widget property can be used to identify a widget as a mega-widget?
3. ... ?

The current ticket, and future Tk development ought be able to rely and build upon that information. The information should not be subject to opinions, fashion whims, etc ...

Suppose that this information becomes available, then I foresee the following issues:
3. What exactly are the "problems" that the Tk code comment in Tk_Bind() mentions?
4. How important are these problems? What shows for the importance?
5. How do other toolkits handle these problems?
6. What responsibility should Tk assume compared to the mega-widget in mitigating the presumed problems?

Managing the reaching of consensus, making choices in all of this is an endeavour that I personally am totally unsuited for. I'll drown in the process (actually, I'm feeling a bit lost already now). If we go down that road, I'll need to rely on, and follow the wisdom and managing competency of others (you?) for this.

Another point of view:
Given that the current "solution" is both incomplete, poorly targeted, and at the same time very intrusive on regular widgets:
- what justifies its existence in the current form at all?
- why would it be the concern of the current ticket to search for a better/complete solution?
This argument speaks for simply removing the current "solution", and defer the responsibility for finding a complete solution to another separate ticket. But maybe this is just my procedural incompetency speaking ...

The wiki has some litterature about megawidgets (just search for this term). There is also this very old paper, I think it describes what the focus and events problem is with megawidgets (the proposed solution and corresponding code are not in the current Tk). From a quick skimming in all this I believe the code you propose to remove in your patch contributes to the solving of the issue with megawidgets but does not constitute a complete solution. This means that the problem with megawidgets exists with or without the code subject to removal, in other words it's perhaps not worse (or is only marginally worse) with this code removed. In short: I'm starting to slowly convince myself your patch can be merged.

A complementary approach to my mild conviction would be we consult some megawidget authors, e.g. Csaba Nemethi (tablelist and other megawidgets). Other people recognizing themselves as megawidget authors welcome to chime in. Thanks!

Thanks for reviewing the ticket.

Regarding "Did you analyze/check/test the impact of your patch on megawidgets?":

Well, the problem is that I also don't know where to get a representative mega-widget:
- test bind-13.10 would be the place to look for it, but it doesn't provide one.
- nevertheless, yes I did look for it, a while ago, when I was writing up the ticket text. But I didn't find anything that identifies a mega-widget, and about which there is consensus. Not in the Tk source code and not in Tk extensions. (When I was writing up the text of this ticket, I was curious whether Tk maintainers would demonstrate that I had been overlooking something.)

Thank you Erik for the complete analysis. I cannot detect anything even slightly incorrect to my eyes in what you have written or provided as a patch. That's great!

I have committed your patch as [58a10b35]. Github Actions reveals that the test suite passes with this on all three platforms. So this looks OK to merge.

I have a question, though. The code section that your patch is removing (i.e. the special treatment of events with a NotifyInferior detail field) is present since the beginning of source code management history, which is 1999. The comment describes the concern that this code section is addressing regarding megawidgets. So my question is: Did you analyze/check/test the impact of your patch on megawidgets? I mean: your reasoning and patch look totally correct, but I'm not sure what the consequences could be for megawidgets. That said, I'm not even sure where to find such a megawidget beast in order to test this (although Tk source code contains a file library/megawidget.tcl).

Just edited the link to the Tk source code in section A. to point at release 8.6.13 (instead of tip).

Eric, thanks for the great work. Francois has written on the core list that he will look into your tickets but he needs some time. THank you for some patience, Harald

Dear Erik, thank you for the great ticket and bug analysis including a solution and a test.

My experience changing something with bindings is quite critical, as many applications implement a very complicated dance of bindings.

Here is my experience with this, which finally failed: https://core.tcl-lang.org/tips/doc/trunk/tip/454.md

In addition, this is user visible. It can be classified as a bug, as the behaviour is not documented. But I fear, it will break a couple of scripts as this area is just sensible.

What do you think of merging this only to Tk 9.0 and not 8.6?

Again congratulation and I hope, we will not loose this gem in a discussion. Harald