Tk Source Code

clipboard-4.1, -4.2, -4.4 fail on macOS

==== clipboard-4.1 ClipboardLostSel procedure FAILED
==== Contents of test case:

    clipboard append "Test"
    selection clear -s CLIPBOARD
    clipboard get

---- Test completed normally; Return code was: 0
---- Return code should have been one of: 1
==== clipboard-4.1 FAILED



==== clipboard-4.2 ClipboardLostSel procedure FAILED
==== Contents of test case:

    clipboard append "Test"
    clipboard append -t TEST "Test2"
    selection clear -s CLIPBOARD
    clipboard get

---- Test completed normally; Return code was: 0
---- Return code should have been one of: 1
==== clipboard-4.2 FAILED



==== clipboard-4.4 ClipboardLostSel procedure FAILED
==== Contents of test case:

    clipboard append "Test"
    clipboard append -t TEST "Test2"
    clipboard append "Test3"
    selection clear -s CLIPBOARD
    clipboard get

---- Test completed normally; Return code was: 0
---- Return code should have been one of: 1
==== clipboard-4.4 FAILED

The TkSuspendClipboard stub function was removed in the redux branch
which was then merged into core-8-6-branch and trunk.

I'm unable to test because Tk is currently broken on 10.14, but reviewing the discussion here, I'm inclined to agree with the removal of TkSuspendClipboard, especially since Marc has already done the work to remove and test this call. The reference to "scrap" indicates that it is indeed a legacy function dating back not just to the Carbon / Tk-Aqua port but all the way back to the old non-Unix classic Mac OS.

Kevin? Marc? ping?

My limited ability to test didn't reveal any remaining issue, however I would be much more comfortable if Kevin confirmed because testing this interactively through VNC is a PITA.

About removing TkSuspendClipboard altogether:

- This function originates from a huge commit in 2002 [6b4c1410c5] "macosx-8-4-branch merged into the mainline [tcl patch #602770]" (this patch can be found in the Tcl repository here), and at this time the content of TkSuspendClipboard was quite different from what it is today. In that early version, there are a lot of references to the "scrap".

- The current content (a simple sync of changeCount value) came from the "Merge of TkAqua Cocoa port" in 2009: [05bd0a2c]. The "Side effects" sentence remained unchanged, likely by error.

In short if we don't need this anymore, as it seems, just forget about this function. I'm quite in agreement with your proposal for removal (note: grepping the source tree spits 9 matches, in 5 files).

I may have fixed this with checkin [8412c865].

When Wish starts up, changeCount is set to -1, meaning that anything in the
clipboard should be pastable.  But when the app was deactivated, so that
commands could be typed in the terminal, the applicationDeactivated method
was called, and it was calling TkSuspendClipboard, which was resetting
changeCount to the current value provided by the Pasteboard.  That means that
nothing has changed in the clipboard, so the clipboard is no longer
pastable unless the clipboardActive flag is set. 

When I deleted the call to TkSuspendClipboard things worked as we expect.
And I could see no reason why TkSuspendClipboard should ever be called.

TkSuspendClipboard is a stub function which seems only to be used by the mac
port, and in the mac port it was used in exactly one place, namely in the
applicationDeactivated method.  So if this fix works, we might want to
eliminate TkSuspendClipboard altogether.  I have no idea what its purpose
was supposed to be.  The original description of the side effects, which I
changed, said "The local scrap is moved to the global scrap."  That means
nothing to me.

I do see a problem here, but I think it is not quite what you are describing.

What I see is that text which was in the clipboard before Wish starts up
cannot be pasted.  But if I start Wish, create and pack the text widget, and
then cut some text from another app it works fine.  This makes sense to me
and I agree that it is related to changeCount.  I think that what is happening
is that when Wish first starts up it assigns the state of the PasteBoard to
changeCount.  If no text has been cut since Wish started, tkSelGetSelection
will not detect any change to the PasteBoard, so it will not return anything
unless the clipboardActive flag is set.

I have looked at the fix proposed by Marc.

The redux branch compiles on macOS, and the test suite is 100% OK for file clipboard.test

Interactively testing however, I cannot paste text from other applications than Tk unless I have made a first paste from Tk text.

I run wish, then pack [text .t], select some text from another app and try to paste it in the text widget I have just created --> nothing happens. If I input text in the text widget, copy text from it and paste it in that same text widget, then it works. If now I try again to paste text from another app then it works.

The fix is propably almost OK, perhaps changeCount needs refined management?

OK. Since I reviewed this, I guess I am responsible.

This fix was not wrong. It was just incomplete. It addressed a real issue where macOS Tk was not using the clipboardActive flag correctly and was therefore generating wrong results. But the clipboardActive flag is not affected when other apps modify the clipboard. The tkCheckClipboard method was apparently intended to detect changes to the system clipboard made by other apps. But it does not do that reliably. It does do something, I believe. It is called when the Tk app is activated, and if another app has changed the clipboard while the Tk app was sleeping it will reset the value of the static variable clipboardOwner and it will generate a SelectionClear event. But if no Tk window owns the clipboard at the time that the app is activated, no event is generated.

I think it might work to simply check whether the clipboard got changed by a different app inside TkSelGetSelection, and if so do the appropriate thing. Check-in [c554e32b] implements this. It seems to allow pasting text cut from another app and also to allow the clipboard test to pass.

Unfortunately, clipboards are complicated.

Reopened because the proposed fix breaks pasting test from external applications, see [568827f41c]

Merged to core-8-6-branch and trunk (thanks to Marc Culler for the review).

All these three tests fail because the last statement "clipboard get" returns an empty string instead of the expected error "CLIPBOARD selection doesn't exist or form "STRING" not defined".

An instrumented interactive test session provides some insight:

% clipboard clear
TkClipInit: clipboardActive set to FALSE
Tk_ClipboardClear: clipboardActive is FALSE
XSetSelectionOwner: clipboardActive is FALSE, changing owner
Tk_ClipboardClear: clipboardActive set to TRUE
% clipboard append Test
Tk_ClipboardAppend: clipboardActive is TRUE
% selection clear -selection CLIPBOARD
XSetSelectionOwner: clipboardActive is TRUE
ClipboardLostSel: clipboardActive set to FALSE
% puts "!![clipboard get]!!"
Tk_GetSelection: entering
Tk_GetSelection: selection owned by some other process
TkSelGetSelection: entering
tkProvidePasteboard: clipboardActive is FALSE
TkSelGetSelection: before calling proc, string is !!!!
!!!!
%

(Note that testing this clipboard feature is prone to errors: you must either run the test from the test suite, or type everything by hand (as opposed to copy/pasting) otherwise the clipboard soon gets mangled by the test of the pasted commands).

From the above log I think that testing dispPtr->clipboardActive is missing in the macOS version of TkSelGetSelection, which is therefore my fix proposal [b4c5f163].

With this patch clipboard-4.[124] no longer fail on macOS.