Tk Source Code

Issue:
-----------------------------------------------
ttk::notebook looses control over the content of its tabs, after hiding and adding several tabs. 

in the GUI applied by the script do the following:

    1. select Tab: "Fourth Tab"
    2. press Button: "visibility 001"
    3. press Button: "visibility 002"   -> (hides fourth tab)
    4. select Tab: "Second Tab"   -> original content is missing
    5. select Tab: "Third Tab"   -> original content is missing 


Possible solution provided by Christian Werner:
-----------------------------------------------
    https://www.androwish.org/home/info/5d45cc97061565e0


Script to reproduce the issue:
-----------------------------------------------
    #
pack [ttk::labelframe .lf  -text "ttk::notebook"] -fill x
pack [ttk::notebook  .lf.nb -height 250] -fill x
	.lf.nb add [frame  .lf.nb.f1  -bg white] -text "First tab"
	.lf.nb add [frame  .lf.nb.f2  -bg white] -text "Second tab"
	.lf.nb add [frame  .lf.nb.f3  -bg white] -text "Third tab"
	.lf.nb add [frame  .lf.nb.f4  -bg white] -text "Fourth tab"
	.lf.nb add [frame  .lf.nb.f5  -bg white] -text "Fifth tab"
	.lf.nb add [frame  .lf.nb.f6  -bg white] -text "Sixth tab"
	.lf.nb add [frame  .lf.nb.f7  -bg white] -text "Seventh tab"
	.lf.nb add [frame  .lf.nb.f8  -bg white] -text "Eighth tab"
    #
	.lf.nb select  .lf.nb.f2
    #
ttk::notebook::enableTraversal .lf.nb
    #
pack [button .lf.nb.f1.bt  -text "this is First Tab"   -width 35  -command [list addMessage "from First Button"]]
pack [button .lf.nb.f2.bt  -text "this is Second Tab"  -width 35  -command [list addMessage "from Second Button"]]
pack [button .lf.nb.f3.bt  -text "this is Third Tab"   -width 35  -command [list addMessage "from Third Button"]]
pack [button .lf.nb.f4.bt  -text "this is Fourth Tab"  -width 35  -command [list addMessage "from Fourth Button"]]
pack [button .lf.nb.f5.bt  -text "this is Fifth Tab"   -width 35  -command [list addMessage "from Fifth Button"]]
pack [button .lf.nb.f6.bt  -text "this is Sixth Tab"   -width 35  -command [list addMessage "from Sixth Button"]]
pack [button .lf.nb.f7.bt  -text "this is Seventh Tab" -width 35  -command [list addMessage "from Seventh Button"]]
pack [button .lf.nb.f8.bt  -text "this is Eighth Tab"  -width 35  -command [list addMessage "from Eighth Button"]]
    #
pack [frame  .f_left]   -side left
    #
pack [frame  .f_left.f_top]
    #
pack [frame  .f_left.f_top.f_left]    -side left
pack [button .f_left.f_top.f_left.bt_v01      -text "visibility 001"  -width 10  -command [list notebookVisibility_001]]      -side top
pack [button .f_left.f_top.f_left.bt_v02      -text "visibility 002"  -width 10  -command [list notebookVisibility_002]]      -side top
pack [button .f_left.f_top.f_left.bt_v03      -text "visibility 003"  -width 10  -command [list notebookVisibility_003]]      -side top
pack [button .f_left.f_top.f_left.bt_v04      -text "visibility 004"  -width 10  -command [list notebookVisibility_004]]      -side top
pack [button .f_left.f_top.f_left.bt_v05      -text "visibility 005"  -width 10  -command [list notebookVisibility_005]]      -side top
pack [button .f_left.f_top.f_left.bt_v99      -text "visibility 099"  -width 10  -command [list notebookVisibility_999]]      -side top
pack [button .f_left.f_top.f_left.bt_all      -text "visibility all"  -width 10  -command [list notebookVisibility_all]]      -side top
    #
pack [frame  .f_left.f_top.f_center]  -side left
pack [button .f_left.f_top.f_center.bt_s01    -text "select First"    -width 10  -command [list notebookSelect  .lf.nb.f1]]   -side top
pack [button .f_left.f_top.f_center.bt_s02    -text "select Second"   -width 10  -command [list notebookSelect  .lf.nb.f2]]   -side top
pack [button .f_left.f_top.f_center.bt_s03    -text "select Third"    -width 10  -command [list notebookSelect  .lf.nb.f3]]   -side top
pack [button .f_left.f_top.f_center.bt_s04    -text "select Fourth"   -width 10  -command [list notebookSelect  .lf.nb.f4]]   -side top
pack [button .f_left.f_top.f_center.bt_s05    -text "select Fifth"    -width 10  -command [list notebookSelect  .lf.nb.f5]]   -side top
pack [button .f_left.f_top.f_center.bt_s06    -text "select Sixth"    -width 10  -command [list notebookSelect  .lf.nb.f6]]   -side top
pack [button .f_left.f_top.f_center.bt_s07    -text "select Seventh"  -width 10  -command [list notebookSelect  .lf.nb.f7]]   -side top
pack [button .f_left.f_top.f_center.bt_s08    -text "select Eighth"   -width 10  -command [list notebookSelect  .lf.nb.f8]]   -side top
    #
pack [frame  .f_left.f_top.f_right]   -side right
pack [button .f_left.f_top.f_right.bt_h01     -text "hide First"      -width 10  -command [list notebookHide    .lf.nb.f1]]   -side top
pack [button .f_left.f_top.f_right.bt_h02     -text "hide Second"     -width 10  -command [list notebookHide    .lf.nb.f2]]   -side top
pack [button .f_left.f_top.f_right.bt_h03     -text "hide Third"      -width 10  -command [list notebookHide    .lf.nb.f3]]   -side top
pack [button .f_left.f_top.f_right.bt_h04     -text "hide Fourth"     -width 10  -command [list notebookHide    .lf.nb.f4]]   -side top
pack [button .f_left.f_top.f_right.bt_s05     -text "hide Fifth"      -width 10  -command [list notebookHide    .lf.nb.f5]]   -side top
pack [button .f_left.f_top.f_right.bt_s06     -text "hide Sixth"      -width 10  -command [list notebookHide    .lf.nb.f6]]   -side top
pack [button .f_left.f_top.f_right.bt_s07     -text "hide Seventh"    -width 10  -command [list notebookHide    .lf.nb.f7]]   -side top
pack [button .f_left.f_top.f_right.bt_s08     -text "hide Eighth"     -width 10  -command [list notebookHide    .lf.nb.f8]]   -side top
   #
pack [frame  .f_left.f_bottom]
    #
pack [button .f_left.f_bottom.bt_status  -text "Current Status"     -width 30  -command [list notebookStatus]]                -side top
pack [button .f_left.f_bottom.bt_visible -text "select 1st visible" -width 30  -command [list selectFirstVisible]]            -side top
pack [button .f_left.f_bottom.bt_clear   -text "clear Text"         -width 30  -command [list clearText]]                     -side top
pack [button .f_left.f_bottom.bt_failure -text "-- failure --"      -width 30  -command [list notebookFailure_MacOS]]         -side top
    #
pack [frame  .f_right]  -side right
pack [text   .f_right.txt]
    #
    #
pack [text   .lf.nb.f1.txt]
pack [text   .lf.nb.f2.txt]
pack [text   .lf.nb.f3.txt]
pack [text   .lf.nb.f4.txt]
pack [text   .lf.nb.f5.txt]
pack [text   .lf.nb.f6.txt]
pack [text   .lf.nb.f7.txt]
pack [text   .lf.nb.f8.txt]
    #
set textManual "please follow these instructions:\n   1. select Tab: \"Fourth Tab\"\n   2. press Button: \"visibility 001\"\n   3. press Button: \"visibility 002\"\n   4. select Tab: \"Second Tab\"\n   5. select Tab: \"Third Tab\""
    #
.lf.nb.f1.txt insert end  "This is Tab: 1\n\n$textManual"
.lf.nb.f2.txt insert end  "This is Tab: 2\n\n$textManual"
.lf.nb.f3.txt insert end  "This is Tab: 3\n\n$textManual"
.lf.nb.f4.txt insert end  "This is Tab: 4\n\n$textManual"
.lf.nb.f5.txt insert end  "This is Tab: 5\n\n$textManual"
.lf.nb.f6.txt insert end  "This is Tab: 6\n\n$textManual"
.lf.nb.f7.txt insert end  "This is Tab: 7\n\n$textManual"
.lf.nb.f8.txt insert end  "This is Tab: 8\n\n$textManual"
    #
proc notebookFailure_MacOS {} {
    # notebookSelect .lf.nb.f3
    notebookSelect .lf.nb.f4
    # notebookVisibility_001
    notebookVisibility_002
}
    #
proc notebookVisibility_all {} {
        #
    addMessage "... notebookVisibility_099"
        #
    set tabDict {
        1 {add  {f1 f2 f3 f4 f5 f6 f7 f8}}
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookVisibility_999 {} {
        #
    addMessage "... notebookVisibility_099"
        #
    set tabDict {
        1 {hide {f1 f2 f3 f4 f5 f6 f7 f8}}
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookVisibility_001 {} {
        #
    addMessage "... notebookVisibility_001"
        #
    set tabDict {
        1 {add  {f1 f2 f3 f4}} 
        2 {hide {f5 f6 f7 f8}} 
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookVisibility_002 {} {
        #
    addMessage "... notebookVisibility_002"
        #
    set tabDict {
        1 {hide {f1}} 
        2 {add  {f2 f3}} 
        3 {hide {f4}}
        4 {add  {f5 f6 f7 f8}}
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookVisibility_003 {} {
        #
    addMessage "... notebookVisibility_003"
        #
    set tabDict {
        1 {hide {f1}} 
        2 {add  {f2 f3 f4}} 
        3 {hide {f5 f6 f7 f8}}
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookVisibility_004 {} {
        #
    addMessage "... notebookVisibility_004"
        #
    set tabDict {
        1 {hide {f1 f2 f3}} 
        2 {add  {f4 f5 f6 f7 f8}}
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookVisibility_005 {} {
        #
    addMessage "... notebookVisibility_005"
        #
    set tabDict {
        1 {hide {f1 f2 f3 f4}} 
        2 {add  {f5 f6 f7 f8}}
    }
        #
    update_NotebookContent $tabDict
        #
}
    #
proc notebookHide {tab} {
    .lf.nb hide $tab
    set retValue [.lf.nb select]
    addMessage "... notebookSelect: current Tab: $retValue"
}
    #
proc notebookSelect {tab} {
    .lf.nb select $tab
    set retValue [.lf.nb select]
    addMessage "... notebookSelect: current Tab: $retValue"
}
    #
proc notebookStatus {} {
    set currentTab  [.lf.nb select]
    addMessage "... notebookStatus: current Tab: [.lf.nb select]"
}
    #
proc update_NotebookContent {dict} {
        #
    dict for {key tabDict} $dict {
        dict for {action tabs} $tabDict {
            switch $action {
                add {
                    addMessage "       ... add:  $tabs"
                    foreach tab $tabs { 
                        .lf.nb add  .lf.nb.$tab
                    }
                }
                hide {
                    addMessage "       ... hide: $tabs"
                    foreach tab $tabs { 
                        .lf.nb hide .lf.nb.$tab
                    }
                }
            }
        }
    }
        #
}
    #
proc selectFirstVisible {} {
    set currentTab  [.lf.nb select]
    addMessage "... selectFirstVisible: current Tab: [.lf.nb select]"
    if {$currentTab == {}} {
        set listActive {}
        addMessage "\n    ... no tab selected!\n"
        foreach tab [.lf.nb tabs] {
            set tabState [.lf.nb tab $tab -state]
            addMessage "     -> $tab -state:$tabState"
            if {$tabState == {normal}} {
                lappend listActive $tab
                addMessage "    01 \$listActive $listActive"
                break
            }
        }
        addMessage "    02 \$listActive $listActive"
        if {$listActive != {}} {
            set activeTab   [lindex $listActive 0]
            addMessage "\n    ... select first normal tab: $activeTab\n"
            .lf.nb select $activeTab
            notebookStatus
        }
    }
}
    #
proc clearText {} {
        #
    .f_right.txt delete 1.0 end
        #
}
    #
proc addMessage {text} {
        #
    puts "$text"
        #
    .f_right.txt insert end "$text\n"
        #
}
    #

Many thanks to Marc Culler and Steve Landers for their feedback on positive testing!

The fix in branch bug-91ca777b4d-alt has now been merged in core-8-6-branch and trunk.

Hi François,

I think this looks good.  I was able to produce a segfault in the widget demo
by opening a menu while the pendulum demo was running.  But I was not able to
reproduce the crash in a second run and I think it is unlikely to be caused by
the changes in this branch.  So I think you can go ahead with the merge.

Thanks, Steve, for the report.

Thank you for the feedback!

Marc, is your testing revealing anything wrong with branch bug-91ca777b4d-alt ? Can I merge this branch and close the present ticket?

I can confirm the fix works and I've removed the workarounds.  Many thanks to all involved.

Of course I will help you with the testing François.  But the more the merrier.

To be specific, I would really like to hear from Steve Landry whether this
allows him to remove all of the workarounds that he was forced to add to his
large proprietary application.

To ease testing I have now created an alternative bugfix branch: bug-91ca777b4d-alt. In this alt branch, I have completely removed TkMacOSXHandleMapOrUnmap() and all flavours of Tk on all platforms use Tk_HandleEvent() to immediately service MapNotify/UnmapNotify events when mapping or unmapping a window. In this alt bugfix branch no test from the test suite is failing for me (I'm on Catalina with a remote connection).

When it comes to interactive testing, I can confirm that the issue reported in the present ticket is solved. Also, I don't see any issues with the Tk widget demoes. Finally it seems the alt bugfix branch didn't resurrect the original issue reported by bll on 2020-09-13 15:08:47 in [6b51f22bff]. As a reminder this issue was:

c) I switch to a different notebook tab.
   I switch back to the first tab.
   The widgets are not redrawn.
   I move the mouse and the widgets get redrawn.

However I'm not 100% sure it's not here because Marc later said in that same ticket on 2020-09-14 22:21:34 that it does not happen every time.

Also, let me remind I can only test on the mac through VNC and this can make a difference for such interactive testing. I would like someone physically in front of a mac to confirm three things with this alt bugfix branch:

• The issue reported in the present ticket does not happen
• The original issue from bll does not happen
• The Tk widget demoes do not show any problem

I need help here. Any taker, please?

If I can get the above confirmations I would merge bug-91ca777b4d-alt rather than bug-91ca777b4d.

Hi François, that is great news!  Both things - that the function is already
disabled for X11 and that your simplification does not create any test failures.  Wow!

I would say that the next step would be to go through the widget demo and
see if you see anything weird.

> try removing it and seeing which tests start to fail

I have just tried changing TkMacOSXHandleMapOrUnmap() in such a way that it always executes Tk_HandleEvent(event) , nothing else and nothing more, and have launched the test suite on macOS. Result: exactly zero test is failing.

But as you mentioned, this code:

> is there for a reason

What should I do to test whether this reason is still valid or not?

Note: The above was tried on a macOS aqua build. TkMacOSXHandleMapOrUnmap() (already) is a mac-aqua-only function. It is not used in X11 (--disable-aqua) builds on the mac. Other platforms only do Tk_HandleEvent(event). See #define TkpHandleMapOrUnmap(tkwin, event) in the three files tk[platform]Port.h. Therefore I think I don't understand your note below about compilation directives that would need to be added.

Hi Christian.  Thanks! I am not offended.

The real can of worms, of course, is the way that Apple changed how they
use the drawRect method in Mojave.  The TkMacOSXHandleMapOrUnmap() function
(and a huge amount of other stuff) was forced by that.

I am pretty sure TkMacOSXHandleMapOrUnmap() is not needed in any system that
resembles X11 and it may well cause lots of trouble in that context. At the
time when I added that I naively thought that I was only dealing with Aqua.
There should probably be some conditional compilation directives which cause
the old XMapWindow / XUnmapWindow code to be used with X11-ish window
managers.

Marc, I must admit, that I've tried the fix exclusively with my AndroWish
branch which is still at 8.6.10 with many cherry picked changes and which
regarding the macosx directory is way behind even core-8-6-branch mostly
due to my not up-to-date (old not updateable) Apple hardware. So it could
be some fundamental platform difference to Win32 and X11 (and my SDL port
which resembles X11) and that's why I've (wrongly?) introduced that evil
term "can of worms". No offense intended at all.

Before declaring it to be a "can of worms" causing lots of trouble you
should definitely try removing it and seeing which tests start to fail.
Many things have changed in the mac event loop since that code was added.
So it is quite possible that it is no longer needed.  On the other hand,
it is there for a reason.  I would advise some caution.

Well this is a very good question indeed, that striked me a long time ago as well. I know at least two issues:

• when the special case in TkMacOSXHandleMapOrUnmap is removed, one gets more or less repeatable crashes in textDisp-5.2, and less repeatably in textwind.test. Try it...! I have started discussing this with Marc Culler four months ago following the fix I have proposed in [61e0bb8aab] but this didn't go anywhere despite I could exhibit a detailed ASan stack trace.

• I strongly suspect the sporadic failures of panedwindow-23.30 on the mac at Github Actions are due to this specific map/unmap event handling. Granted, I don't have a final proof for this one though.

Whoa, François! Great explanation indeed. Now I wonder what huge can of
worms the map/unmap event handling on the Mac still imposes on other
geometry managing widgets and commands.

I think I am now at the bottom of this. On macOS there is a difference with respect to the other platforms as far as MapNotify and UnmapNotify events are concerned. This is all explained in TkMacOSXHandleMapOrUnmap() (macosx/tkMacOSXWm.c), and in much more details in comments by Marc Culler that can be found in ticket [6b51f22bff], specifically the two dated 2020-09-24 17:00:14 and 2020-09-27 21:03:44. Thanks to Marc, who took the time to write down everything he did and the reasons why he did it, I think I understand now the issue in the present ticket. In a nutshell, it's exactly what chw said below (but without being able to pinpoint where it happens in the code): "on the Mac some additional loopery concerning framework specific event handling takes places (which it does not on other platforms) which requires to have a proper initial state (no more current tag) before calling Ttk_UnmapContent()". TkMacOSXHandleMapOrUnmap() processes the event it receives by queing it at the tail of the event queue, and repeatedly servicing this event queue, filtering and servicing MapNotify and UnmapNotify events only. When several MapNotify/UnmapNotify are already waiting in the queue, this processing is not the same as only immediately handling (Tk_HandleEvent) the new MapNotify/UnmapNotify event.

All this explains why chw's fix works. I believe we can now merge the bugfix branch.

(Final note for the records: I can also fix the present ticket by making TkMacOSXHandleMapOrUnmap() always run Tk_HandleEvent(event) and do nothing else, and this also concurs in my understanding of the issue. But of course we cannot do this otherwise we would resurrect issues dealt with in the above mentioned ticket [6b51f22bff].)

I have also seen this problem and have been discussing it with Marc Culler. My suspicion is that a raise is happening before a window is mapped, and that doesn't work on macOS.  So is that a generic problem or a macOS problem or a macOS problem that needs to be fixed in the generic code?

You can workaround the problem by raising the notebook tab window in the <<NotebookTabChanged>> callback ...

bind .lf.nb <<NotebookTabChanged>> notebookChanged

proc notebookChanged {} {
	set index [.lf.nb index current]
	set win [lindex [.lf.nb tabs] $index]
	puts stderr "raise $win"
	raise $win
}

I did try to fix this in generic/ttk/ttkNotebook.c SelectTab() by adding Tk_RestackWindow(nb->core.tkwin, Above, NULL) just before the Tk_SendVirtualEvent but that didn't work.  So the problem may be more complex than raising before mapping. But it is in that general area.

My humble theory is that on the Mac some additional loopery
concerning framework specific event handling takes places
(which it does not on other platforms) which requires to have a
proper initial state (no more current tag) before calling
Ttk_UnmapContent(). The other patch snippet of [251223db]
is cosmetics since SelectNearestTab() implicitely dispatches
for redisplay. It could be left out.

I have investigated this a bit more.

First, here is a somewhat minimal script showing the inconsistent view (Tab 1 selected, but the displayed content is from Tab 2):

package require Tk
pack [ttk::notebook .nb -height 50] -fill x
for {set i 1} {$i <= 2} {incr i} {
    .nb add [frame .nb.f$i] -text "Tab $i"
    pack [text .nb.f$i.txt]
    .nb.f$i.txt insert end  "Displaying Tab $i"
}
.nb select .nb.f2
update idletasks ; after 200
# next line triggers the problem
.nb add .nb.f1 ; .nb hide .nb.f2

This shows the problem on macOS but not on Windows.

The fix proposed by chw definitely works. But so far I couldn't figure out why. And I couldn't figure out why this fix is not needed on Windows.

I am totally sure we are (at least: I am) missing something here. I'm not comfortable with merging the change, I must understand first.

I have now committed your (modified) proposed fix in [251223db].

Could we have a small test script demonstrating the bug? We could then turn it into a non-gregression test.

I'm still puzzled by the fact I cannot reproduce on Windows whereas the fix is in the generic code. I think we should understand this to have full confidence in the proposed fix. How can we explain this oddity?

Moreover, even if the currently provided test case doesn't demonstrate it, shouldn't need to add:

	nb->notebook.currentIndex = -1;

fvogel, you're right, the test failures are introduced by the change in
TabRemoved(). This was not my brightest idea since it's plain wrong.
Leave it out and the initial problem still seems to be fixed.

Another observation: while the proposed fix seems to actually resolve the issue observed on macOS, it also introduces two new failures (notebook-6.4 and notebook-6.6). I didn't look at whether the expected results of these tests could be wrong though.

The fix proposes to change generic code. However, if the problem was in the generic code, I should be able to reproduce it on any platform. But I do not reproduce on Windows at least, which makes me slightly dibious about this fix.