Tk Source Code

View Ticket
Login
Ticket UUID: 2513186
Title: ttk::entry xview sub-command forces use of update idletasks
Type: Bug Version: obsolete: 8.5.5
Submitter: cjmcdonald Created on: 2009-01-16 12:55:37
Subsystem: 88. Themed Tk Assigned To: fvogel
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2019-05-08 08:44:07
Resolution: Fixed Closed By: fvogel
    Closed on: 2019-05-08 08:44:07
Description:
Before the ttk::entry xview sub-command is invoked, an
update idletasks command must be executed in order to
update the entry scrolling information, otherwise the
sub-command doesn't work correctly e.g. the following
script is supposed to set an entry field value and
then scroll it to the end ready for editing:

    pack [ttk::entry .e -width 8]
    .e insert end abcdefghijklmnopqrstuvwxyz
    .e xview [.e index end]

The field is left unscrolled, unless update idletasks
is inserted before the xview sub-command.

The use of update and update idletasks is generally
discouraged on comp.lang.tcl, unless *absolutely
necessary*, for well known reasons.  Recursively
re-entering the event loop can lead to application
logic problems, the widget may have been destroyed,
etc.  I don't think that it is desirable for a core
widget to force use of update idletasks just to
function correctly.  It is particularly a nuisance if
a whole form of entry fields is being set ready for
modification.


The following change to generic/ttk/ttkEntry.c will
avoid the problem:

static int EntryXViewCommand(
    Tcl_Interp *interp, int objc, Tcl_Obj *const objv[], void *recordPtr)
{
    Entry *entryPtr = recordPtr;
    
    /* If caller has requested that the widget view is
     * adjusted to a given index "pathName xview index",
     * then ensure that the scroll info is up-to-date.
     */
     
    if (objc == 3) {
EntryDoLayout(recordPtr);
    }

    return TtkScrollviewCommand(interp, objc, objv,
        entryPtr->entry.xscrollHandle);
}


Proc ttk::entry::See in library/ttk/entry.tcl can then
also be modified to remove an update idletasks.
User Comments: fvogel added on 2019-05-08 08:44:07:
Bugfix branch merged into core-8-6-branch and trunk. Thanks for your help in testing and for the original idea of the patch!

cjmcdonald added on 2019-04-30 10:18:24:
I've tested the bug fix branch with my applications, and all is good.  Thank you for your thorough investigation of the issue.

fvogel added on 2019-04-28 14:47:29: (text/x-fossil-wiki)
The root cause of the issue reported here is that the widget redisplay happens as an idle task. This redisplay runs the layoutProc and the displayProc of the widget. It is in the layoutProc that the scroll info are set. Therefore the wrong answer when requesting xview or yview of this widget before idle tasks had a chance to run. Adding 'update idletasks' between the widget changes and the xview command was so far the way to handle this, but as mentioned this has drawbacks (namely: *all* idle tasks are run, not just the one we're interested in here).

The original patch proposed to call the layoutProc directly each time xview is executed for the ttk::entry widget. This was a bit of an overkill. I have now reworked the patch to make it minimally invasive, that is to avoid calling the layoutProc when it was not stricly necessary, thus saving performance (it is only necessary when there is a pending redisplay of the widget, at other times the scroll info are already up-to-date). Also, I made the patch deal with all Ttk scroll commands for all Ttk widgets, not just xview for the ttk::entry. Finally I have added new tests checking the fix.

All this is in branch [https://core.tcl-lang.org/tk/timeline?r=bug-2513186fff|bug-2513186fff]. Please try it out and report your findings here.

fvogel added on 2019-04-27 15:22:41: (text/x-fossil-wiki)
Same issue here: [8261c517af]

fvogel added on 2019-04-22 22:17:55: (text/x-fossil-wiki)
Patch committed in [b6f1c7d8a5]. I have slightly modified it since the problem was not only apparent when calling <code>.e xview $ind</code>, but it was present with all possible variations of the xview command. Any of the following four scripts failed to provide the expected result before the patch:

<verbatim>
package require Tk
pack [ttk::entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview [.e index end]

package require Tk
pack [ttk::entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview

package require Tk
pack [ttk::entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview moveto 0.7

package require Tk
pack [ttk::entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview scroll 5 units
</verbatim>

However, the Tk entry widget does not suffer from the same problemn i.e. any of the following four scripts already works as expected:

<verbatim>
package require Tk
pack [entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview [.e index end]

package require Tk
pack [entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview

package require Tk
pack [entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview moveto 0.7

package require Tk
pack [entry .e -width 8]
.e insert end abcdefghijklmnopqrstuvwxyz
.e xview scroll 5 units
</verbatim>

Obviously, ttk test entry-3.2 now passes without its embedded 'update idletasks'. I'll look at adding further non-regression tests.

fvogel added on 2019-04-22 21:34:19: (text/x-fossil-wiki)
X-ref to [f9343d8f72], where this topic was addressed again. The solution from there, based on binding to <Expose> looks like a desperate hack. I would very much prefer the solution proposed in the present ticket be adopted. I'll give this ticket a try, for ttk::entry, and for (Tk) entry as well.