Tk Source Code

TkSelGetSelection requests an X selection conversion. According to the ICCCM section "Requesting a Selection",

Requestors should ensure that the named property does not exist on the window before issuing the ConvertSelection request. The exception to this rule is when the requestor intends to pass parameters with the request (see below).

Since TkSelGetSelection provides no way to supply a parameter for the conversion request, it should delete the property before making the request. Otherwise, the current value of the property (which is the result of the previous conversion request for the same selection) may be unintentionally interpreted by the selection owner as the parameter.

Thank you for your answers, that's fine with me.

I have now merged your patch to core-8-6-branch and trunk.

I have checked that this change does not modify the results from the test suite (I tested this in core-8-6-branch, modulo the usual unstable test results).

Closing this ticket now, many thanks for your input and time helping.

1. According to the ICCCM,

Requestors should not use None for the property argument of a ConvertSelection request.

The reason is that the selection owner would then have to choose a property itself, and there is no way for it to make that choice safely.

2. I originally found the issue and made the fix as part of an attempt to implement the fd.o clipboard manager specification, which makes use of conversion request parameters. I eventually abandoned the attempt, as it became clear that it would require more effort than I could spare, but I thought I should at least submit the fix.

Admittedly, the fix is more theoretical than practical. I only know of two selection targets that use parameters: MULTIPLE from the ICCCM, and SAVE_TARGETS from the aforementioned clipboard manager spec. MULTIPLE can't be usefully requested with Tk, because it requires a parameter. SAVE_TARGETS's parameter is optional, and so it would actually be a decent test case, but by its nature an application shouldn't need to use it more than once. But I suppose if an application does use it more than once, then it will be affected by the bug, so that's something.

I see your argumentation and patch, thanks.

I have committed the patch to branch bug-28d0b8fb2f for evaluation (I didn't test it yet).

I have two questions for you:

1. The patch in fact evaluates:

    retr.property = selection;
[...]
    XDeleteProperty(winPtr->display, retr.winPtr->window, retr.property);

    retr.property = None;

2. The code you propose to patch has been here and such for ages, at least since 1998, with no problem I know of. Could you perhaps sketch a test case that would demonstrate the problem that the patch intends to fix?