Tk Source Code

View Ticket
Login
Ticket UUID: 1591493
Title: textImage-3.2 test case failure
Type: Bug Version: obsolete: 8.5a5
Submitter: lvirden Created on: 2006-11-06 17:13:03
Subsystem: 18. [text] Assigned To: fvogel
Priority: 7 High Severity: Minor
Status: Closed Last Modified: 2015-02-15 19:23:01
Resolution: Fixed Closed By: fvogel
    Closed on: 2015-02-15 19:23:01
Description:
tk 8.5 cvs head built on solaris 2.9 with sun c
compiler, test suite display back via exceed 10.0.0.12
on win xp.

textImage.test

==== textImage-3.2 delayed image management FAILED
==== Contents of test case:

        catch {
            image create photo small -width 5 -height 5
            small put red -to 0 0 4 4
        }
        catch {destroy .t}
        text .t -font test_font -bd 0
-highlightthickness 0 -padx 0 -pady 0
        pack .t
        .t image create end -name test
        update
        set result ""
        lappend result [.t bbox test]
        .t image configure test -image small -align top
        update
        lappend result [.t bbox test]

---- Result was:
{0 8 0 0} {0 0 5 5}
---- Result should have been (exact matching):
{} {0 0 5 5}
==== textImage-3.2 FAILED
User Comments: fvogel added on 2015-02-15 19:23:01:
test textImage-3.2 is now fixed in trunk and core-8-5-branch.

fvogel added on 2015-02-15 17:40:47:
Further observations:

I. Regarding textDisp-32.2
   -----------------------

1. What textDisp-32.2 is checking definitely happens when visually looking at the text widget at the different phases of the testing steps in textDisp-32.2, i.e. -lmargin1 correct indentation happens or not depending on relevant tag elision. In other words, the expected behavior in textDisp-32.2 definitely happens.

2. The way of testing this behavior is through bbox-ing indices 1.0 and "1.0 + 0 displaychars". The latter adjusts to a non-elided index before being passed to bbox, and because of that does not fail. The former however provides an elided index to bbox. Before [b26b042c68] this returned {}, i.e. bbox returned {} when the index it is passed to is elided. After [b26b042c68] bbox returns a non empty list when the index it is passed to is both elided and on screen.

Actually the question boils down to whether 'bbox $elidedIndexOnScreen' should return {} or a non-empty list.

The rationale for the change of bbox behavior and for the associated non-{} test results in textDisp-32.2 was (see discussion in bug 1288677):

"'bbox' is defined to return an empty string for things
that are not visible which I would interpret as not on
screen.  If they are on screen but of zero width, then bbox
should return that. (There are easy ways to determine if
something is elided if one wants to test that, but it would
be bad to confuse tests which want to know if something is
onscreen or offscreen)."

I concur with this statement.

Which means that commit [b26b042c68] is correct.


II. Regarding textImage-3.2
    -----------------------

Commit [b26b042c68] then makes textImage-3.2 fail because this test uses the empty list result of bbox to check for "delayed image management".

I try to imagine what "delayed image management" means. Perhaps what one wants to check in that test is that even if an image index was created (.t image create) this index does not trigger a relayout/redraw process until this index gets mapped to an actual image (.t image configure -image).

But such a behavior does not happen. As far as I know it never happened. I tried by checking $tk_textRelayout and $tk_textRedraw as follows:

package require Tk
image create photo small -width 5 -height 5
small put red -to 0 0 4 4
text .t -bd 0 -highlightthickness 0 -padx 0 -pady 0
pack .t
.t debug on
set res {}
update
.t image create end -name test
update
lappend res $tk_textRelayout $tk_textRedraw
.t image configure test -image small -align top
update
lappend res $tk_textRelayout $tk_textRedraw

This returns:  1.0 1.0 1.0 1.0
whereas it should return:  {} {} 1.0 1.0
if delayed image management would happen.
I tried this with a source tree at [2accaed23b], which is the last GOOD commit having textImage-3.2 succeed.

Also looking at the source code, when creating an image in the text widget, TkTextChanged is called irrespective if the index refers to a configured -image or not. And TkTextChanged triggers relayout/redraw.

This non-occurring "delayed image management" behaviour was wrongly tested in textImage-3.2 through the bbox command which in itself has nothing to do with delayed management.

What could be tested instead in textImage-3.2 is the following behavior:
1. An image name gets created in the text widget:
    .t image create end -name myname
2. This image has an associated index in the text widget but this index takes no display space at this step: bbox output is a non-empty list reporting zero width.
3. An image gets associated to the image name:
    .t image configure myname -image $xxx
4. The index corresponding to the image name does not change x and y field, but now it gets a non-zero width in the bbox output

I will rewrite textImage-3.2 to achieve the above testing goal.

fvogel added on 2015-02-15 14:01:58:
Reverting the following part of [b26b042c68] restores textImage-3.2 to working status:


Index: generic/tkTextDisp.c
==================================================================
--- generic/tkTextDisp.c
+++ generic/tkTextDisp.c
@@ -7118,23 +7118,12 @@
     } else {
        if (charWidthPtr != NULL) {
            *charWidthPtr = *widthPtr;
        }
     }
-    if (*widthPtr == 0) {
-       /*
-        * With zero width (e.g. elided text) we just need to make sure it is
-        * onscreen, where the '=' case here is ok.
-        */
-
-       if (*xPtr < dInfoPtr->x) {
-           return -1;
-       }
-    } else {
-       if ((*xPtr + *widthPtr) <= dInfoPtr->x) {
-           return -1;
-       }
+    if ((*xPtr + *widthPtr) <= dInfoPtr->x) {
+        return -1;
     }
     if ((*xPtr + *widthPtr) > dInfoPtr->maxX) {
        *widthPtr = dInfoPtr->maxX - *xPtr;
        if (*widthPtr <= 0) {
            return -1;


But then textDisp-32.2 fails, since this test was designed precisely to check that the code removed above is working (see bug 1288677):

---- Result was:
{1.0 20 20} {1.29 {} 0} {1.0 0 0} {1.29 {} 20} {1.0 20 20} {1.29 {} 20} {1.0 20 20}
---- Result should have been (exact matching):
{1.0 20 20} {1.29 0 0} {1.0 0 0} {1.29 0 20} {1.0 20 20} {1.29 0 20} {1.0 20 20}
==== textDisp-32.2 FAILED

fvogel added on 2015-02-15 13:12:54:
fossil diff --to b26b042c68  --from 2accaed23b ChangeLog

spits:

--- ChangeLog
+++ ChangeLog
@@ -1,5 +1,29 @@
+2005-10-10  Vince Darley <[email protected]>
+
+       * doc/text.n
+       * generic/tkText.c
+       * generic/tkText.h
+       * generic/tkTextBTree.c
+       * generic/tkTextDisp.c
+       * generic/tkTextImage.c
+       * generic/tkTextIndex.c
+       * generic/tkTextMark.c
+       * generic/tkTextTag.c
+       * generic/tkTextWind.c
+       * macosx/tkMacOSXDefault.h
+       * tests/text.test
+       * tests/textDisp.test
+       * unix/tkUnixDefault.h
+       * win/tkWinDefault.h: Implementation of TIP#256, adding a new
+       text widget configuration option '-tabstyle', with new tests
+       and documentation.
+
+       Also a fix for [Bug 1281228] (documentation and full implementation
+       of -strictlimits), and [Bug 1288677] (corrected elide behaviour),
+       again with more tests.

I like this very much: A single commit containing implementation of a TIP, and two unrelated bug fixes.

This does not help maintainability.

fvogel added on 2015-02-15 13:03:33:
fossil bisect points at the following commit:

uuid:         b26b042c687a34c89798f90861d4035a87ee6212 2005-10-10 10:36:34 UTC
parent:       2accaed23b8019511c7ce0b039eda6da60e7843b 2005-10-05 04:14:10 UTC
child:        4b125f71b22213800c258c84185bf3a45bc9dd3c 2005-10-10 19:28:27 UTC
leaf:         no
tags:         trunk
comment:      tip256 implementation (user: vincentdarley)

fvogel added on 2014-10-28 22:30:11:

I believe that this test failure is the same as the issue described here:

http://core.tcl.tk/tk/tktview?name=1581955fff

And I'm inclined to agree with dkf on that matter (see his comment in the above ticket):

"Bluntly, there's not much chance of fixing this [...]. My suspicion/hunch is that it'll be hard to fix because we now need the dimensions of the embedded windows much earlier so that we can get smooth scrolling right."


fvogel added on 2014-10-28 22:29:24:

I believe that this test failure is the same as the issue described here:

http://core.tcl.tk/tk/tktview?name=1581955fff

And I'm inclined to agree with dkf on that matter (see his comment in the above ticket):

"Bluntly, there's not much chance of fixing this [...]. My suspicion/hunch is that it'll be hard to fix because we now need the dimensions of the embedded windows much earlier so that we can get smooth scrolling right."


fvogel added on 2014-10-28 21:37:20:
Not specific to Solaris.

This test fails for me as well, on Windows and on Linux Debian 6.0 Squeeze, in both core-8-5-branch and trunk.

On Linux Debian, the tests results are as shown by the OP.

On Windows the results are (in both branches):

---- Result was:
{0 10 0 0} {0 0 5 5}
---- Result should have been (exact matching):
{} {0 0 5 5}
==== textImage-3.2 FAILED

lvirden added on 2008-10-15 01:12:38:
Just an update here. 2 years later. Using Tk 8.5.4 released version.
Running against SPARC Solaris 8, displaying back to windows xp sp 3 using exceed's 10.0.0.12 version.

I see the identical error message.

lvirden added on 2007-10-16 20:53:26:
Logged In: YES 
user_id=15949
Originator: YES

This identical behavior continues to manifest with the latest tcl/tk 8.5 beta cvs head.