Bug 15407 - tk_select.list(): double-click on scrollbar gives error
tk_select.list(): double-click on scrollbar gives error
Status: CLOSED FIXED
Product: R
Classification: Unclassified
Component: Graphics
R 3.0.1
x86_64/x64/amd64 (64-bit) Linux
: P5 enhancement
Assigned To: R-core
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-29 05:20 UTC by Scott Kostyshak
Modified: 2013-11-12 02:40 UTC (History)
2 users (show)

See Also:


Attachments
solves problem by removing double-click feature (1.63 KB, patch)
2013-07-29 05:22 UTC, Scott Kostyshak
Details | Diff
a different approach (1.12 KB, patch)
2013-07-29 05:24 UTC, Scott Kostyshak
Details | Diff
patch by Peter (1.02 KB, patch)
2013-07-29 09:59 UTC, Scott Kostyshak
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Kostyshak 2013-07-29 05:20:25 UTC
I can reproduce the following two unexpected behaviors on Ubuntu 12.04, 12.10, and 13.04, with R version 3.0.1 and with trunk revision r63436.


(1)
a. execute install.packages()
b. double-click on the scrollbar.

result: I get an error box with title "Application Error" and contents <<Error: invalid command name ".1.4">> (note that the number changes depending on where in the scrollbar I double-click). Expanding the box by clicking on "Details" gives the following:

invalid command name ".1.4"
invalid command name ".1.4"
    while executing
"$w cget -command"
    (procedure "Scroll" line 2)
    invoked from within
"Scroll .1.4 1 pages"
    (in namespace inscope "::ttk::scrollbar" script line 1)
    invoked from within
"::namespace inscope ::ttk::scrollbar {Scroll .1.4 1 pages}"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 $Repeat(script)"
    (procedure "ttk::Repeat" line 3)
    invoked from within
"ttk::Repeat"
    ("after" script)

Note also that double-clicking on the scrollbar arrows or any blank space in the box executes an OK.


(2)
a. execute install.packages()
b. single-click on an item.
c. double-click on that same item.

result:
R gives the following error:
"Error in install.packages() : no packages were specified"

I expected the item that was double-clicked to be returned (regardless of whether it was already selected).


Below is my session info:
> sessionInfo()
R Under development (unstable) (2013-07-28 r63436)
Platform: x86_64-unknown-linux-gnu (64-bit)

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base
Comment 1 Scott Kostyshak 2013-07-29 05:22:48 UTC
Created attachment 1465 [details]
solves problem by removing double-click feature

I do not know how to bind the double-click to onOK only for the items in the selection box. tcltk_va.patch removes the double-click binding (globally). It fixes both unexpected behaviors (1) and (2) at the cost of removing the double-click feature. It partially reverts r50641 and (guessing from that commit description but not tested) thus creates an inconsistency between Unix and Windows.
Comment 2 Scott Kostyshak 2013-07-29 05:24:17 UTC
Created attachment 1466 [details]
a different approach

If double-click behavior should be preserved, the attached patch, tcltk_vb.patch, is my attempt. It fixes situations (1) and (2) but has the following problem:

(3)
a. execute install.packages()
b. single-click on an item.
c. double-click on the scrollbar.

result:
There is an error (similar to the one in (1)) and the item that was clicked is returned.


In my opinion tcltk_vb.patch leads to better behavior than the current code because I imagine that (3) occurs less often than (1).
Comment 3 Peter Dalgaard 2013-07-29 08:18:40 UTC
I see this on OSX too. (Snow Leopard if it matters).

I think the double click on scrollbar issue is caused by binding the event to the container instead of the listbox. I.e. in

    tkbind(dlg, "<Double-ButtonPress-1>", onOK)

we  should have box, not dlg. (I could test but Scott seems to be poking around already.)

If I understand correctly, what happens is that the double-click triggers both the dlg binding and the scrollbar binding in sequence, but onOK destroys dlg and box, and then the scrollbar tries to scroll the destroyed box.

The double click on selected item issue could be trickier: The first click deselects the item in the listbox and the second triggers onOK. If this doesn't change if the double press is bound to the listbox (it might), some tk tinkering is required.
Comment 4 Scott Kostyshak 2013-07-29 09:59:06 UTC
Created attachment 1467 [details]
patch by Peter
Comment 5 Scott Kostyshak 2013-07-29 10:03:58 UTC
(In reply to comment #3)
> I think the double click on scrollbar issue is caused by binding the event to
> the container instead of the listbox. I.e. in
> 
>     tkbind(dlg, "<Double-ButtonPress-1>", onOK)
> 
> we  should have box, not dlg. (I could test but Scott seems to be poking around
> already.)

You are right. I attached the patch as tcltk_vc.patch. I tested it.

> If I understand correctly, what happens is that the double-click triggers both
> the dlg binding and the scrollbar binding in sequence, but onOK destroys dlg
> and box, and then the scrollbar tries to scroll the destroyed box.

Thanks for the explanation. This explains why an error was generated when the scrollbar was double-clicked but why no error was generated when a blank space was double-clicked.

> The double click on selected item issue could be trickier: The first click
> deselects the item in the listbox and the second triggers onOK. If this doesn't
> change if the double press is bound to the listbox (it might), some tk
> tinkering is required.

Let me know if I should open another bug report and if there is more testing I can do.
Comment 6 Scott Kostyshak 2013-10-27 04:45:31 UTC
I've been testing the patch (most recently on r64110) and it works well. Note that another commonly used function affected by this bug is chooseCRANmirror().
Comment 7 Peter Dalgaard 2013-11-11 13:37:53 UTC
Fix for (1)  is now committed in R-devel and R-patched. Part (2) is trickier and may not be fixable (it is generally considered bad GUI design if double-click semantics are not extensions of single-clicks, and the first click will deselect if the item under the cursor is already selected.)

I'm closing the PR  now, please open a new request if you come up with a good fix for part (2).
Comment 8 Scott Kostyshak 2013-11-12 02:40:40 UTC
(In reply to Peter Dalgaard from comment #7)
> Fix for (1)  is now committed in R-devel and R-patched. Part (2) is trickier
> and may not be fixable (it is generally considered bad GUI design if
> double-click semantics are not extensions of single-clicks, and the first
> click will deselect if the item under the cursor is already selected.)

Thanks for putting it in, Peter.

> I'm closing the PR  now, please open a new request if you come up with a
> good fix for part (2).

I have no plans to look into (2).