Tk Source Code

View Ticket
Login
Ticket UUID: 7703f947aa6682d6cc057798f2029d50bef3c24e
Title: Many many issues with elided lines in the text widget
Type: Bug Version: trunk
Submitter: fvogel Created on: 2014-11-22 12:31:53
Subsystem: 18. [text] Assigned To: fvogel
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2015-02-06 21:27:34
Resolution: Fixed Closed By: fvogel
    Closed on: 2015-02-06 21:27:34
Description:
Many many issues with elided lines in the text widget.


The following is a first issue, see the comments below in this ticket for other failing test cases.



Wrong refresh of display lines when tagging text as elided:


package require Tk
catch {destroy .t}
pack [text .t]
for {set i 1} {$i < 5} {incr i} {
  #              0          1    
  #              012345 678901234
  .t insert end "Line $i+++Line $i\n"
}
.t tag configure hidden -elide true
.t tag add hidden 2.6 3.6

.t tag add hidden 3.11 4.6

# --> Elision is visually not OK !!!
# The text widget looks like this:
#
#     Line 1+++Line 1
#     Line 2+++Line 3
#     +++Line 4

.t tag configure hidden -elide true

# --> Now it's OK!
# The text widget looks like this:
#
#    Line 1+++Line 1
#    Line 2+++Li+++Line 4
User Comments: fvogel added on 2015-02-06 21:27:34:
Branch text-elided containing the fixes for all bugs mentioned in this ticket has now been merged to core-8-5-branch and trunk.

kevin_walzer added on 2015-01-04 23:40:18:
I have just committed an update to tkTextDisp.c on trunk and core-8-5-branch that fixes some Mac-specific scrolling issues: please be sure to preserve those edits when you commit your final work from this branch. Thank you.

fvogel added on 2014-12-28 21:03:35:
Further fixed (in branch text-elided) 'text see' with indices in elided lines, since previous fixes such as [5f352f3a71] were not correct in all cases.

Now ALL calls to CalculateDisplayLineHeight use an index that is at the start of a display line, as requested by CalculateDisplayLineHeight.

Test case 'L':
--------------
package require Tk
pack [text .t]
pack configure .t -expand 1 -fill both
for {set i 1} {$i < 50} {incr i} {
    .t insert end "Line $i\n"
}
# button just for having a line with a larger height
button .t.b -text "Test" -bd 2 -highlightthickness 2
.t window create 21.0 -window .t.b
wm geometry . 300x200+0+0
.t tag add hidden 15.36 21.0
.t tag configure hidden -elide true
# Indices 21.0, 17.0 and 15.0 are all on the same display line
# therefore index @0,0 shall be the same for all of them
.t see end
update
.t see 21.0
update
set ind1 [.t index @0,0]
.t see end
update
.t see 17.0
update
set ind2 [.t index @0,0]
.t see end
update
.t see 15.0
update
set ind3 [.t index @0,0]
list [expr {$ind1 == $ind2}] [expr {$ind1 == $ind3}] ; correct is {1 1}

fvogel added on 2014-12-28 18:09:34:
Further fixed (in branch text-elided) 'text count -ypixels' with indices in elided lines, since previous fixes such as [30d6b995dc] were not correct in all cases (sigh!).

Test case 'K':
--------------

package require Tk
pack [text .t]
for {set i 1} {$i < 25} {incr i} {
    .t insert end [string repeat "Line $i -" 10]
    .t insert end "\n"
}
.t debug on
wm geometry . 200x200+0+0
.t tag add hidden 5.36 11.0
.t tag configure hidden -elide true
.t yview 5.0
update
.t count -ypixels 5.0 11.0  ; # returns : 0 ; correct : 16 (1 line)

fvogel added on 2014-12-28 15:14:49:
Fixed (in branch text-elided) bad counting of the total number of vertical pixels in the text widget, resulting in small change of the Y scrollbar size. Happened because CalculateDisplayLineHeight expects an index at start of a display line, which was not always the case.

Test case 'J':
--------------

package require Tk
entry .e2 -justify center -width 50
pack .e2
pack [text .t -width 30 -yscroll ".s set"] [scrollbar .s -command ".t yview"] -side left -fill y
pack configure .t -expand 1 -fill both
for {set i 1} {$i < 25} {incr i} {
    .t insert end "Line $i\n"
}
wm geometry . 300x200+0+0
.t tag add hidden 5.7 11.0
.t tag configure hidden -elide true
update
set y1 [lindex [.t yview] 1]
.t count -displaylines 5.0 11.0   ; # makes scrollbar change size!
set y2 [lindex [.t yview] 1]
.t count -displaylines 5.0 12.0   ; # makes scrollbar size OK again!
set y3 [lindex [.t yview] 1]
list [expr {$y1 == $y2}] [expr {$y1 == $y3}]  ; # shall be {1 1}

fvogel added on 2014-12-17 21:58:55:
Fixed MeasureUp when there are elided lines.

Test case 'I':
--------------

package require Tk
pack [text .t -width 30 -yscroll ".s set"] [scrollbar .s -command ".t yview"] -side left -fill y
pack conf .t -expand 1 -fill both
.t tag config hidden -elide 1
wm geometry . 450x200+0+0
foreach x [list 0 1 2 3 4 5 6 7 8 9 0] {
  .t insert end "$x aaa1\n$x bbb2\n$x ccc3\n$x ddd4\n$x eee5\n$x fff6"
  .t insert end "$x 1111\n$x 2222\n$x 3333\n$x 4444\n$x 5555\n$x 6666" hidden
}
.t see 23.0
.t index @0,0  ; # Returns 3.0 but shall be 1.0


To try all this, see branch text-elided.

fvogel added on 2014-12-10 22:16:06:
BTW, the fix for TkTextChanged now makes the cursor correctly blink when set on a line containing elided chars.

Test case 'H':
--------------

package require Tk
pack [text .t]
for {set i 1} {$i < 10} {incr i} {
    .t insert end "Line $i - This is Line [format %c [expr 64+$i]]\n"
}
.t tag add hidden 6.8 7.17
.t tag configure hidden -background red
.t tag configure hidden -elide true
wm geometry . 300x200+0+0
.t mark set insert 7.18  ; # insert cursor not seen or does not blink
focus -force .t
.t mark set insert 7.20  ; # insert cursor not moved, not seen and does not blink
focus -force .t

fvogel added on 2014-12-10 22:08:46:
Fixed last caller of FindDLine: TextChanged now correctly takes into account of elided newlines.

See branch text-elided that contains all fixes regarding elided text in the text widget.

fvogel added on 2014-12-07 18:59:45:
Fixed text yview scroll pixels|lines with elided lines. Typical test case:

Test case 'G':
--------------

package require Tk
pack [text .t]
for {set i 1} {$i < 100} {incr i} {
    .t insert end [string repeat "Line $i" 20]
    .t insert end "\n"
}

.t configure -wrap none
.t tag add hidden 5.15 20.15
.t tag configure hidden -elide true

update

.t yview 35.0
.t yview scroll -240 pixels
.t index @0,0     ; # shall be 5.0 (not 20.0)

.t yview 35.0
.t yview scroll -15 units
.t index @0,0     ; # shall be 5.0 (not 20.0)


[text yview scroll xx pixels] and [text yview scroll xx lines] with elided lines is now fixed in branch bug-7703f947aa, with new non-regression tests added (textDisp-16.42, textDisp-16.43 and textDisp-19.18)

fvogel added on 2014-12-05 23:44:16:
Test case 'F' for text count -ypixels with indices in elided lines is now fixed in branch bug-7703f947aa, with new non-regression test added for this case (textDisp-19.17)

fvogel added on 2014-12-05 21:51:01:
Another command not properly taking into account elided lines is [text count -ypixels]

Test case 'F':
--------------

package require Tk
text .t
pack .t
for {set i 1} {$i < 100} {incr i} {
    .t insert end [string repeat "Line $i" 20]
    .t insert end "\n"
}
.t configure -wrap none
.t tag add hidden 5.15 20.15
.t tag configure hidden -elide true
.t count -ypixels 1.0 6.0    ; # returns 80 ; correct: 64 (4 * lineheight)
.t count -ypixels 2.0 7.5    ; # returns 64 ; correct: 48 (3 * lineheight)
.t count -ypixels 5.0 8.5    ; # returns 16 ; correct: 0
.t count -ypixels 6.1 6.2    ; # returns 0  ; correct: 0
.t count -ypixels 6.1 18.8   ; # returns 0  ; correct: 0
.t count -ypixels 18.0 20.50 ; # returns 0  ; correct: 0
.t count -ypixels 5.2 20.60  ; # returns 16 ; correct: 0
.t count -ypixels 20.60 20.70; # returns 0  ; correct: 0
.t count -ypixels 5.0 25.0   ; # returns 80 ; correct: 80 (5 * lineheight)
.t count -ypixels 25.0 5.0   ; # returns -80 ; correct: -80 (- 5 * lineheight)
.t count -ypixels 25.4 27.50 ; # returns 32 ; correct: 32 (2 * lineheight)


TkTextIndexYPixels needs to be fixed in that respect.

fvogel added on 2014-12-05 20:40:57:
Fixed text count -xpixels with indices in elided lines. Typical test case:

Test case 'E':
--------------

package require Tk
text .t
pack .t
for {set i 1} {$i < 100} {incr i} {
    .t insert end [string repeat "Line $i" 20]
    .t insert end "\n"
}
.t configure -wrap none
.t tag add hidden 5.15 20.15
.t tag configure hidden -elide true
update
.t count -xpixels 5.15 6.0 ; # returns -120 ; correct: 0
.t count -xpixels 5.15 6.1 ; # returns -112 ; correct: 0
.t count -xpixels 6.0 6.1  ; # returns 8    ; correct: 0
.t count -xpixels 6.1 6.2  ; # returns 8    ; correct: 0
.t count -xpixels 6.1 6.0  ; # returns -8   ; correct: 0
.t count -xpixels 6.0 7.0  ; # returns 0    ; correct: 0
.t count -xpixels 6.1 7.1  ; # returns 8    ; correct: 0
.t count -xpixels 15.0 20.15  ; # returns 120 ; correct: 0
.t count -xpixels 20.15 20.16 ; # returns 0   ; correct: 8 (1 charwidth)
.t count -xpixels 20.16 20.15 ; # returns 0   ; correct: -8 (-1 charwidth)


[text count -xpixels] with elided lines is now fixed in branch bug-7703f947aa, with new non-regression test added for this case (textDisp-16.41)

fvogel added on 2014-12-03 23:36:46:
Fixed text see command with target indices in elided lines. The typical test case is:

Test case 'D':
--------------

package require Tk
text .t
pack .t
for {set i 1} {$i < 100} {incr i} {
    .t insert end [string repeat "Line $i" 20]
    .t insert end "\n"
}
.t configure -wrap none
.t tag add hidden 5.2 20.3
.t tag configure hidden -elide true

update

.t see 9.0  ; # --> wrong scroll up: index 9.0 is already visible


[text see] with elided lines is now fixed in branch bug-7703f947aa, with new non-regression test added for this case (textDisp-11.18)

fvogel added on 2014-12-03 07:07:12:
Fixed embedded windows tests that were failing following [dc9f4bea9f].


textWind-10.4.1, 10.5, 10.6, 10.6.1, 11.2 and 14.4 were failing as follows after [dc9f4bea9f]:


==== textWind-10.4.1 EmbWinLayoutProc procedure, error in creating window FAILED

==== Contents of test case:

    .t delete 1.0 end
    .t insert 1.0 "Some sample text"
    catch {destroy .t.f}
    .t window create 1.5 -create {
        frame .t.f
        frame .t.f.f -width 10 -height 20 -bg $color
    }
    set msg {}
    update idletasks
    lappend msg [winfo exists .t.f.f]

---- Result was:
{{window name "f" already exists in parent}} {{can't embed .t.f.f relative to .t
}} 1
---- Result should have been (exact matching):
{{can't embed .t.f.f relative to .t}} 1
==== textWind-10.4.1 FAILED



==== textWind-10.5 EmbWinLayoutProc procedure, error in creating window FAILED
==== Contents of test case:

    .t delete 1.0 end
    .t insert 1.0 "Some sample text"
    .t window create 1.5 -create {
        concat .t
    }
    set msg {}
    update
    lappend msg [.t bbox 1.5]

---- Result was:
{{can't embed .t.f.f relative to .t}} {{can't embed .t relative to .t}} {40 12 0
 0}
---- Result should have been (exact matching):
{{can't embed .t relative to .t}} {40 12 0 0}
==== textWind-10.5 FAILED



==== textWind-10.6 EmbWinLayoutProc procedure, error in creating window FAILED
==== Contents of test case:

    .t delete 1.0 end
    .t insert 1.0 "Some sample text"
    catch {destroy .t2}
    .t window create 1.5 -create {
        toplevel .t2 -width 100 -height 150
        wm geom .t2 +0+0
        concat .t2
    }
    set msg {}
    update
    lappend msg [.t bbox 1.5]

---- Result was:
{{can't embed .t relative to .t}} {{can't embed .t2 relative to .t}} {{window na
me "t2" already exists in parent}} {40 12 0 0}
---- Result should have been (exact matching):
{{can't embed .t2 relative to .t}} {{window name "t2" already exists in parent}}
 {40 12 0 0}
==== textWind-10.6 FAILED



==== textWind-10.6.1 EmbWinLayoutProc procedure, error in creating window FAILED

==== Contents of test case:

    .t delete 1.0 end
    .t insert 1.0 "Some sample text"
    catch {destroy .t2}
    .t window create 1.5 -create {
        toplevel .t2 -width 100 -height 150
        wm geom .t2 +0+0
        concat .t2
    }
    set msg {}
    update
    set i 0
    while {[llength $msg] == 1 && [incr i] < 200} { update }
    set msg

---- Result was:
{{window name "t2" already exists in parent}} {{can't embed .t2 relative to .t}}

---- Result should have been (exact matching):
{{can't embed .t2 relative to .t}} {{window name "t2" already exists in parent}}

==== textWind-10.6.1 FAILED



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

    .t delete 1.0 end
    .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

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: window name "f" already exists in parent
    while executing
"frame .t.f -width 30 -height 20 -bg $color"
    ("uplevel" body line 6)
    invoked from within
"uplevel 1 $script"
---- errorCode: NONE
==== textWind-11.2 FAILED



==== textWind-14.4 EmbWinDelayedUnmap procedure FAILED
==== Contents of test case:

    .t delete 1.0 end
    .t insert 1.0 "Some sample text\nAnother line\n3\n4\n5\n6\n7\n8\n9"
    frame .t.f -width 30 -height 20 -bg $color
    .t window create 1.2 -window .t.f
    update
    .t yview 2.0
    set result [winfo ismapped .t.f]
    update
    list $result [winfo ismapped .t.f]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: window name "f" already exists in parent
    while executing
"frame .t.f -width 30 -height 20 -bg $color"
    ("uplevel" body line 4)
    invoked from within
"uplevel 1 $script"
---- errorCode: NONE
==== textWind-14.4 FAILED



This commit fixed FindDLine so that it really provides the correct display line to its caller, in all cases including fully elided lines. The fix makes use of TkTextFindDisplayLineEnd to check, in FindDLine, if the desired index is on the last display line of the text widget or not. The previous version of FindDLine did not call TkTextFindDisplayLineEnd.

It turns out that calling TkTextFindDisplayLineEnd launches the couple LayoutDLine/FreeDLine, which has the undesired effect of mapping and unmapping the embedded windows laying on the last (in this call conditions) display line of the widget.

In turn, this triggers the differences in the results of the tests mentioned above: the complementary errors returned are due to this added mapping/unmapping of embedded windows triggered when looking if the index is on the last display line of the widget.

This call to LayoutDLine/FreeDLine is indeed documented in textDisp.c:3365-3370.
I believe this was not considered as a bug so far, rather as a room for improvement regarding performance. Therefore I have fixed the failing tests (sprinkled a few updates here and there) and kept TkTextFindDisplayLineEnd unchanged.

This is what [13d2fcd25d] does.

Note that not all embedded windows tests are exercised on my platform (Windows), adn that some other tests may fail on other platforms. Just let me know and I'll fix them, if any.

fvogel added on 2014-11-30 22:04:55:
Test case 'C' now fixed in branch bug-7703f947aa, with new non-regression test added for this case (textDisp-22.11)

More will follow, stay tuned.

fvogel added on 2014-11-30 21:55:25:
Again a failing test case:


Test case 'C':
--------------

package require Tk
text .t
pack .t
.t configure -wrap char
for {set i 1} {$i < 10} {incr i} {
  .t insert end "Line $i - Line _$i - Lines .$i - Line [format %c [expr 64+$i]]\n"
}
.t tag add hidden 1.30 2.5
.t tag configure hidden -elide true
wm geometry . 300x200+0+0
update

.t bbox 2.4  ; # returns {}, it should not!

fvogel added on 2014-11-30 21:24:59:
Test case 'B' now fixed in branch bug-7703f947aa, with new non-regression test added for this case (textDisp-22.10)

fvogel added on 2014-11-24 21:21:21:
Another failing thing that needs fixing (problem is in chunk identification in TkTextIndexBbox):


Test case 'B':
--------------

package require Tk
text .t
pack .t
for {set i 1} {$i < 10} {incr i} {
    .t insert end "Line $i - This is Line [format %c [expr 64+$i]]\n"
}
.t tag add hidden 2.8 2.17
.t tag add hidden 6.8 7.17
.t tag configure hidden -elide true
update

.t bbox 2.0
# {2 18 8 16}   -> OK  (for the font I have)
.t bbox 2.7
# {58 18 8 16}  -> OK
.t bbox 2.8
# {66 18 0 0}   -> OK - Same correct result up to  .t bbox 2.17
.t bbox 2.18
# {74 18 8 16}  -> OK

.t bbox 6.0
# {2 82 8 16}   -> OK
.t bbox 6.7
# {58 82 8 16}  -> OK
.t bbox 6.8
# {66 82 0 0}   -> OK - Same correct result up to lineend
#                  i.e.  .t bbox 2.23
.t bbox 7.0
# {}            -> WRONG (char is on screen) - Same wrong result 
#                 till .t bbox "7.0 lineend" ; Should be {66 82 0 0} 
.t bbox 8.0
# {2 98 8 16}   -> OK


Looking at this...

fvogel added on 2014-11-23 14:17:59:
Fixing FindDLine is not enough in all cases. It's callers shall be fixed as well.

Test case 'A':
--------------

package require Tk
catch {destroy .t}
pack [text .t]
for {set i 1} {$i < 10} {incr i} {
    .t insert end "Line $i - This is Line [format %c [expr 64+$i]]\n"
}
.t tag add hidden 2.8 2.17
.t tag add hidden 6.8 7.17
.t tag configure hidden -background red
.t tag configure hidden -elide true
# so far, so good: display is OK

.t tag configure hidden -elide false
# the second tag range is not refreshed (i.e. it's still elided)


Fixed that in branch bug-7703f947aa, with new non-regression test added for this case (textDisp-9.13)

fvogel added on 2014-11-22 22:29:26:
Fixed in branch bug-7703f947aa

New non-regression test added for this bug (textDisp-9.12)

One test failed after the fix: textDisp-9.6
After analysis it turns out that the expected test results for this test were not relevant without updating between tag add and tag remove (see thed etails of this test).
I've fixed the test, and we have now correct expected test results for textDisp-9.6. And this test passes.

In addition there was the same issue for textDisp-9.3, -9.4, and -9.5 although the test suite did not reveal it (they did not fail after the fix for this bug). Fixed that as well.

fvogel added on 2014-11-22 12:39:53:
When calling    .t tag add hidden 3.11 4.6
the code wants to launch a refresh starting on just the affected display lines. Problem is that the first display line picked by findDLine for index 3.11 is the wrong one due to the elided eol between lines 2 and 3.

When calling    .t tag configure hidden -elide true
the code launches a refresh on all lines of the text widget, which explains why the widget becomes visually correct then.