Tk Source Code

View Ticket
Login
Bounty program for improvements to Tcl and certain Tcl packages.
2016-11-07
07:55 Ticket [2863003f] Add a virtual event when GM leaves parent without slaves status still Open with 4 other changes artifact: 7a730ff9 user: oehhar
07:53 Closed ticket [d6b95ce4]: tk frame does not shrink to 1 height if last children unpacked plus 8 other changes artifact: cbef1b68 user: oehhar
2016-10-27
09:09 Ticket [d6b95ce4]: 4 changes artifact: 419aa2a3 user: oehhar
2016-10-22
17:11 Ticket [2863003f] Add a virtual event when GM leaves parent without slaves status still Open with 4 other changes artifact: 42263a48 user: oehhar
2016-09-22
17:07 Ticket [d6b95ce4] tk frame does not shrink to 1 height if last children unpacked status still Open with 3 other changes artifact: b10da450 user: oehhar
16:48 Ticket [d6b95ce4]: 4 changes artifact: 719f96a3 user: oehhar
13:18 Ticket [d6b95ce4]: 3 changes artifact: 87c97b0e user: emiliano
09:17 Ticket [d6b95ce4]: 4 changes artifact: 1b7aafb3 user: oehhar
09:16
Patch by Koen in Ticket [d6b95ce4]: no check for ALLOCED_MASTER, second place in grid where master can become empty check-in: bd3aa3c9 user: oehhar tags: tip-454
08:49 Ticket [d6b95ce4] tk frame does not shrink to 0 height if last children unpacked status still Open with 3 other changes artifact: 9aef0b63 user: danckaert
2016-09-21
14:50 Ticket [d6b95ce4]: 4 changes artifact: e1515e54 user: oehhar
13:48 Ticket [d6b95ce4]: 3 changes artifact: ab9927aa user: fvogel
13:47
grid: shrink frame when last child is ungridded. Ticket [d6b95ce4] check-in: 318549b3 user: fvogel tags: tip-454
09:12 Ticket [d6b95ce4] tk frame does not shrink to 0 height if last children unpacked status still Open with 4 other changes artifact: 883f2a9e user: oehhar
09:11 Ticket [d6b95ce4]: 3 changes artifact: c710b8c5 user: danckaert
08:51 Ticket [d6b95ce4]: 4 changes artifact: 05a071cb user: oehhar
08:44
pack: shrink frame when last child is unpacked. Ticket [d6b95ce4] check-in: 64fe74c3 user: oehhar tags: bug-d6b95ce492-alt, tip-454
08:39 Ticket [d6b95ce4] tk frame does not shrink to 0 height if last children unpacked status still Open with 4 other changes artifact: 2a25171f user: oehhar
07:25 Ticket [d6b95ce4]: 4 changes artifact: e4df76b8 user: oehhar
07:13 Ticket [2863003f] Add a virtual event when GM leaves parent without slaves status still Open with 7 other changes artifact: 7ba1fc07 user: oehhar
06:32
(Modernized) patch from egavilan to fix [2863003fff] and [d6b95ce492] - tk frame does not shrink to 0 height if last children unpacked. A virtual event is sent when the pack or grid geometry manager leaves a parent without any slaves Leaf check-in: 60230550 user: fvogel tags: bug-d6b95ce492, rfe-2863003fff
2016-09-20
13:01 New ticket [d6b95ce4] tk frame does not shrink to 0 height if last children unpacked. artifact: 35a41381 user: oehhar

Ticket UUID: d6b95ce49207c8231f1ac9cfd7391f0748c73ce1
Title: tk frame does not shrink to 1 height if last children unpacked
Type: Bug Version: 8.6.6
Submitter: oehhar Created on: 2016-09-20 13:01:56
Subsystem: 48. Geometry Management Assigned To: nobody
Priority: 5 Medium Severity: Important
Status: Closed Last Modified: 2016-11-07 07:53:56
Resolution: Wont Fix Closed By: oehhar
    Closed on: 2016-11-07 07:53:56
Description:

Issue

An empty packed or gridded (tk/ttk/label)frame has zero height and width:
pack [frame .f]
When children are packed, it resizes to its size:
pack [entry .f.e]
When the last children is unpacked, the frame widget keeps the size of the last child:
pack forget .f.e
IMHO it should resize to 0 size. IMHO this is a bug.

Workarounds

The page http://wiki.tcl.tk/frame contains some work-arounds.

Discussion

On the core list, there is a discussion thread about this issue: http://code.activestate.com/lists/tcl-core/16363/ with an explanatory post by Joe English.

Solutions

Emiliano has a solution sketch in this Ticket: [2863003fff].

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

User Comments: oehhar added on 2016-11-07 07:53:56:

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.


oehhar added on 2016-10-27 09:09:46:

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.
     */
The current patch for TIP 454 bypasses this by doing the Tk_GeometryRequest() immediately instead of at idle time. The result is that another geometry manager can still take over, but it introduces some flickering (collapse + expand):
  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

The core list archive does not have the messages jet, so:
  • 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


oehhar added on 2016-09-22 16:48:45:

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


emiliano added on 2016-09-22 13:18:41:
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.

oehhar added on 2016-09-22 09:17:49:

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


danckaert added on 2016-09-22 08:49:44:

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;
     }


oehhar added on 2016-09-21 14:50:22:

'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


fvogel added on 2016-09-21 13:48:59:
Propagated Koen's patch from pack to the grid geometry manager.

See branch bug-d6b95ce492-alt

oehhar added on 2016-09-21 09:12:16:

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


danckaert added on 2016-09-21 09:11:51:

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

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


oehhar added on 2016-09-21 08:51:48:

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


oehhar added on 2016-09-21 08:39:13:

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;
     }


oehhar added on 2016-09-21 07:25:39:

Discussion now in ticket [2863003f].