Issue

pack [frame .f]

pack [entry .f.e]

pack forget .f.e

Workarounds

Discussion

Solutions

Looking to the discussion, this feals like a broader issue.

Nevertheless, I find this issue quite annoying for dynamic setups and the workarounds all have issues not allowing a smooth solution.

Thank you all, Harald

In addition to the last message: Koens post: http://code.activestate.com/lists/tcl-core/16544/ I did not find Brians post.

Now, the TIP is decided as withdrawn. The issue may be fixed by patch in ticket [2863003f] (add an additional event).

I close this bug as discussion may continue there.

After the positive vote of TIP 454, the following issues are reported on the core list.

Koen Dankart: flicker

I have to say though that I'm getting less sure about this TIP. I found some comments in the code indicating that the old behaviour was not so much a design choice, but rather an implementation issue. Specifically, this comment in tkGrid.c:1735.

    /*
     * If the master has no slaves anymore, then don't do anything at all:
     * just leave the master's size as-is. Otherwise there is no way to
     * "relinquish" control over the master so another geometry manager can
     * take over.
     */

  pack [text .t]
  pack forget .t; grid .t
  grid forget .t; pack .t

Brian Griffin: Additional Configure-Event

It turns out that this "flicker" is also what is causing our tests to hang. Our UI is a complex set of nested and tabbed panes where the implementation that manages the panes relies on a complex dance of <Configure>, <Map> and <Unmap> events to make the right things happen. The consequence of adding one more <Configure> event is causing a hang at a [tkwait visibility $win] where $win never appears. $win is a single child in a frame that is unmanaged and (re)managed based on various conditions.

The hang is easily solved, but that means that the behavior is different. (the difference is not right or wrong, just different*) This difference is also demonstrated in the textWind failures. It could be asserted that these tk tests are confirming that the comment in the tkGrid.c has been faithfully implemented.

I cannot write just one piece of code that works in both 8.6.6 and 8.6.7. I contend that this sort of thing is forbidden in a minor patch release, and questionable in a major patch relase (e.g. 8.7). I'm struggling with this, but I think this kind of change might have to be deferred to 9.0.

-Brian

*: the change in our code means removing some code. From a perspective of "less is more", the patch is "better".

Harald Oehlmann: test textWind-11.1 and textWind-11.2

New test failures:

==== textWind-11.1 EmbWinDisplayProc procedure, geometry transforms FAILED
==== Contents of test case:

     .t insert 1.0 "Some sample text"
     pack forget .t
     place .t -x 30 -y 50
     frame .f -width 30 -height 20 -bg $color
     .t window create 1.12 -window .f
     update
     winfo geom .f

---- Result was:
1x1+0+0
---- Result should have been (exact matching):
30x20+119+55
==== textWind-11.1 FAILED



==== textWind-11.2 EmbWinDisplayProc procedure, geometry transforms FAILED
==== Contents of test case:

     .t insert 1.0 "Some sample text"
     pack forget .t
     place .t -x 30 -y 50
     frame .t.f -width 30 -height 20 -bg $color
     .t window create 1.12 -window .t.f
     update
     winfo geom .t.f

---- Result was:
1x1+0+0
---- Result should have been (exact matching):
30x20+89+5
==== textWind-11.2 FAILED

Koens explanation

These 2 tests are failing because after the [pack forget] the window has collapsed to 1x1, as expected. The tests then use [place .t] to put the text widget back, but the [place] window manager does not propagate sizes to its master window. So, to make the tests succeed again, the root window size should be configured before [place], e.g. with [. configure -width 400 -height 200].

Core List References

• Title: RE: [TCLCORE] RE : CFV: TIP #454: Automatically Resize Frames After Last Child Removed
• Date: all 2016-10-26
• Timestamps (GMT): HaO: 8:44 , Koen: 15:53, Brian: 16:46

This bug is now subject of TIP 454 http://tip.tcl.tk/454

I personally don't think the actual behaviour of the propagating
geometry managers or the container widgets are actual bugs, but more
like an overlook in the design.

The problem with the proposed solution of requesting a 1x1 (*) size after the
last managed widget goes away is that it sets a policy, not a mechanism.
People might want it to be left alone, or set to another geometry, or destroy
it, or ... you get the idea.

This is why I proposed the solution of sending a virtual event, so the
user can get a notification and do whatever it wants with the frame in a
few lines of code. I know is not a fully cooked idea, but IMHO is an step
in the right direction. Forcing the geometry to be 1x1 is as bad as what
we have now. Or worse, since it's a potential incompatibility.

Thank you Koen, you are just impressive ! Added to bug-branch by commit [bd3aa3c9].

Here is a revised patch. Apparently the check on DONT_PROPAGATE was not needed, since ALLOCED_MASTER is also not set in that case. In [grid] there are two places where the master can become empty.

diff --git a/generic/tkGrid.c b/generic/tkGrid.c
index 2a88b76..8cb02c9 100644
--- a/generic/tkGrid.c
+++ b/generic/tkGrid.c
@@ -2780,6 +2780,7 @@ Unlink(
      */
 
     if ((masterPtr->slavePtr == NULL) && (masterPtr->flags & ALLOCED_MASTER)) {
+        Tk_GeometryRequest(masterPtr->tkwin, 0, 0);
        TkFreeGeometryMaster(masterPtr->tkwin, "grid");
        masterPtr->flags &= ~ALLOCED_MASTER;
     }
@@ -3507,6 +3508,7 @@ ConfigureSlaves(
      */
 
     if (masterPtr->slavePtr == NULL && masterPtr->flags & ALLOCED_MASTER) {
+        Tk_GeometryRequest(masterPtr->tkwin, 0, 0);
        TkFreeGeometryMaster(masterPtr->tkwin, "grid");
        masterPtr->flags &= ~ALLOCED_MASTER;
     }
diff --git a/generic/tkPack.c b/generic/tkPack.c
index 88a4b2d..f26712a 100644
--- a/generic/tkPack.c
+++ b/generic/tkPack.c
@@ -1363,6 +1363,7 @@ Unlink(
      */
 
     if (masterPtr->slavePtr == NULL && masterPtr->flags & ALLOCED_MASTER) {
+        Tk_GeometryRequest(masterPtr->tkwin, 0, 0);
        TkFreeGeometryMaster(masterPtr->tkwin, "pack");
        masterPtr->flags &= ~ALLOCED_MASTER;
     }

'grid' works for me to:

 % pack [frame .c]
% winfo height .c
1
% grid [entry .c.e]
% winfo height .c
19
% grid forget .c.e
% winfo height .c
1

Great, thank you, Francois, Harald

Propagated Koen's patch from pack to the grid geometry manager.

See branch bug-d6b95ce492-alt

Sorry Koen, for pack it works correctly, thank you:

% pack [frame .f]
% winfo height .f
1
% pack [entry .f.e]
% winfo height .f
19
% pack forget .f.e
% winfo height .f
1

Thank you, that looks simple and great ! Harald

This was just a partial patch for [pack] alone. Try:

  pack [frame .f]
  pack [entry .f.e]
  pack forget .f.e

Koen, I have tried your patch (commit [64fe74c3] in branch [bug-d6b95ce4]).

For me, the frame does not resize to 0x0 after this sequence:

% pack [frame .f]
% grid [entry .f.e]
% grid forget .f.e
(

Sorry, but it does not work for me.

Thanks, Harald

Koen Dankart contributed by private E-Mail:

The question is whether the current behaviour is a bug or not. Some people say that it is not a bug because the frame loses its geometry manager after the last child is unpacked, and it keeps its last computed size. But the geometry manager might as well recompute the size as its last action before disconnecting from the frame. It think the latter behaviour would be less surprising than the current one, which many seem to consider as a bug indeed.

Now, if it is a bug, we can probably find a better fix than delivering a virtual event. In think the following would do it for 'pack':

diff --git a/generic/tkPack.c b/generic/tkPack.c
index 88a4b2d..80cf320 100644
--- a/generic/tkPack.c
+++ b/generic/tkPack.c
@@ -1363,6 +1363,9 @@ Unlink(
      */
 
     if (masterPtr->slavePtr == NULL && masterPtr->flags & ALLOCED_MASTER) {
+        if (!(masterPtr->flags & DONT_PROPAGATE)) {
+            Tk_GeometryRequest(masterPtr->tkwin, 0, 0);
+        }
        TkFreeGeometryMaster(masterPtr->tkwin, "pack");
        masterPtr->flags &= ~ALLOCED_MASTER;
     }

Discussion now in ticket [2863003f].