[prev in list] [next in list] [prev in thread] [next in thread] 

List:       rdesktop-devel
Subject:    [rdesktop-devel] [ rdesktop-Patches-1339819 ] Clipboard: Unicode, XFixes, Obsolete PRIMARY
From:       "SourceForge.net" <noreply () sourceforge ! net>
Date:       2006-03-27 10:22:28
Message-ID: E1FNork-00053X-Vq () sc8-sf-web2 ! sourceforge ! net
[Download RAW message or body]

Patches item #1339819, was opened at 2005-10-27 20:46
Message generated for change (Comment added) made by ikonst
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=381349&aid=1339819&group_id=24366

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Ilya Konstantinov (ikonst)
Assigned to: Erik Forsberg (forsberg)
Summary: Clipboard: Unicode, XFixes, Obsolete PRIMARY

Initial Comment:
This patch takes care of the following:
- Adds Unicode support, for both incoming and outgoing
clipboard data, as long as iconv is available.
- Adds XFixes support on new xorg servers, which allows
us to be notified about clipboard changes and provide
cleaner RDP support (RDP expects those notifications).
Of course, where it's not available, there's an
alternative code path.
- Stops using PRIMARY selection (the one that is
updated when you select text) in favor of CLIPBOARD
selection (the one you use explicitly with copy-paste
commands). Currently rdesktop is using both PRIMARY and
CLIPBOARD, which is confusing, non-standard (both GTK+
and Qt have clear separation between CLIPBOARD and
PRIMARY) and uncomfortable (you select a text you want
replaced with something in the clipboard, and the text
you selected replaces the clipboard text!). See
http://standards.freedesktop.org/clipboards-spec/clipboards-0.1.txt
for more info.
- No longer offer TEXT clipboard format, since we don't
support it properlty (according to the standard) so
better not try at all.

This patch adds the necessary autoconf magic to detect
and use Xfixes when available.

----------------------------------------------------------------------

>Comment By: Ilya Konstantinov (ikonst)
Date: 2006-03-27 12:22

Message:
Logged In: YES 
user_id=335423

That's nice of you.
I'll get to integrating the XFixes part of the patch next
week, hopefully.

----------------------------------------------------------------------

Comment By: Pierre Ossman (ossman_)
Date: 2006-03-27 09:00

Message:
Logged In: YES 
user_id=1469081

Since not much has happened on the command line option part,
I'll be taking over that part. I'll also do a small tweak of
the "auto" mode to look at the timestamp of the selection
and use the most recent one.


----------------------------------------------------------------------

Comment By: Erik Forsberg (forsberg)
Date: 2005-11-07 17:15

Message:
Logged In: YES 
user_id=9251

Well, since all three of us (you, me, Peter) agree on this
solution, and it won't upset Matt's mother :-), Go for it!

----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-07 14:37

Message:
Logged In: YES 
user_id=335423

This decission sounds sensible to me. Do I get the green
light for implementing it? I already begun by posting the
first patch and I'm waiting for it to be reviewed. As we'll
be working incrementally, the time it takes to commit each
patch directly influences the time when I'll begin working
on the next patch.

BTW, OOWriter's selections (not copies) don't go into PRIMARY?!

----------------------------------------------------------------------

Comment By: Erik Forsberg (forsberg)
Date: 2005-11-07 14:26

Message:
Logged In: YES 
user_id=9251

We (Peter Åstrand and I) had a discussion at a whiteboard this
morning, regarding the clipboard functionality in rdesktop,
and more specifially about the use of PRIMARY, CLIPBOARD, or
both. 

Our opinion is that we should add an option to rdesktop for
deciding
how to handle clipboard at runtime. The option could look
something like this:

-r clipboard:[off|PRIMARYCLIPBOARD|CLIPBOARD]

In '-r clipboard:off' mode, clipboard redirection should be
turned
off. This should probably not be the default.

In '-r clipboard:PRIMARYCLIPBOARD' mode, rdesktop should
work exactly as today, first looking at PRIMARY, and if that
is empty, try
CLIPBOARD. This mode should probably be the default.

In '-r clipboard:CLIPBOARD' mode, rdesktop should only look at
CLIPBOARD, ignoring PRIMARY. This mode is for gnome/KDE
applications using rdesktop as a backend. We will probably
use it in ThinLinc as well. 

The rationale behind this thinking is that the
PRIMARYCLIPBOARD mode is actually very functional in a mixed
environment with both KDE/Gnome and legacy applications
which don't understand CLIPBOARD, and it does allow Matt's
mother to copy what she've selected with her mouse into a
Windows program as well. Using only PRIMARY will make cut
nonfunctional for some programs, one example being
Openoffice writer, so we actually need to check CLIPBOARD
after PRIMARY. How applications handle a cut operation seems
very application specific - kate (an editor in KDE) and
oocalc does add cut'ed data to PRIMARY, while oowriter doesn't.

The CLIPBOARD-only mode on the other hand will work very
well in a
modern KDE/Gnome environment, and for systems where most
users have just migrated from a Windows-only environment,
this solution is the best, since it allows for less
confusion when users by mistake make a selection that goes
into PRIMARY after their original copy or cut operation. 

The above should apply only for cut/copy operations in the
X11 ->
Windows direction. For Windows -> X11 cut/copy operations, both
PRIMARY and CLIPBOARD should be used, regardless of mode
(well, if clipboard redirection is turned off, obviously,
neither should be used :-) ). This is to try mimicing both
the 'selected text goes into
PRIMARY' behaviour and the CLIPBOARD behaviour. A text that
has been copied/cut in Windows have been selected with the
mouse or cursor way, so it should be available in PRIMARY,
just as any other selected text in a X11 window. 

We agree with Matt about the large amount of options for
rdesktop , but we think adding this option is good anyway.
It adds useful
functionality, and it also adds symmetry - the other
redirections have their -r, where you can turn on/off and
configure the redirection, so clipboard should have one as well.


----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-04 18:41

Message:
Logged In: YES 
user_id=335423

Allright, I'll try to split it this weekend.

And another thing -- when I said "FreeDesktop-compliant
applications which use rdesktop as a backend would benefit
from being able to make
it use PRIMARY", I really meant to say "use CLIPBOARD",
since CLIPBOARD is the more natural thing when dealing with
modern desktop environments and performing an "explicit
copy" (and all Windows Copy operations are "explicit copy").
Even GNOME and KDE terminals have Edit | Paste menu items
which use CLIPBOARD.

----------------------------------------------------------------------

Comment By: Erik Forsberg (forsberg)
Date: 2005-11-04 17:03

Message:
Logged In: YES 
user_id=9251

Regarding the splitting into smaller patches - yes,
unfortunately I think it's neccessary. It's not good
software maintenance practice to commit this patch in its
current form. I would hate myself later for doing it, when I
need to find out where that little irritating bug sneaked
in, and all there is in the cvs log is "commited gigantic
patch". You know the feeling.. ;-). Small, incremental
patches is the way to go.

You will hate me now, but you'll love me in the long run,
when you remember that irritating guy from Sweden that
taught you the hard way never to create 48k patches again
;-). I'll buy you some of your favourite beverage if we
meet, as a compensation for being such a pain in the
you-know-where =).

I'll comment on the other issues later. Too late friday
afternoon for qualified clipboard thinking right now. 


----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-04 16:06

Message:
Logged In: YES 
user_id=335423

> ... tried to copy stuff from
winword on a w2003 to Openoffice 2.0 beta on a KDE 3.2 on
top of SLES9. Doesn't work

Whoa, I can reproduce it too, and only from Winword (copying
from Notepad works fine). I will add additional debug info
to show what clipboard formats are offered by the RDP side.

> xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=CLIPBOARD

I'd say this is Writer's request. I wonder why it doesn't
like anything we offer as TARGETS.

> 
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION

And that's just some random Qt application? I should add the
window ID of the requestor to the debug otuput.

> xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=COMPOUND_TEXT, property=_QT_SELECTION

I really wonder what does it think its doing. We never
offered COMPOUND_TEXT. Anyhow, this doesn't seem to be
related to the problem.

----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-04 15:28

Message:
Logged In: YES 
user_id=335423

Hi Erik,

I'm really excited to have someone finally take a look at
the patch :)

> First of all, the patch is too big.

I agree. I begun working on Unicode support, but then got
dragged into more and more modifications until I ended up
with the current monster-of-a-patch. By then, the code has
gone through quite a bit of refactoring so artificially
spliting the patch into smaller pieces would take even more
time (which could be spent elsewhere) and would be
superfluous work. I'm sure you know the feeling :)

> Second, I'd like an explanation of what is gained by using
the XFixes extension.

In the original X11 selections model, the client only
receives notifications when it loses its own grip on the
selection, but not when the owner changes. Therefore, it
doesn't know when to re-request TARGETS. That prohibits the
application from knowing when it has available clipboard
data in any of the formats it understand (e.g. to know when
to gray out the Paste menu item). Qt solved it (now I think
it uses XFIXES too) by having a root-window property of
_QT_CLIPBOARD_SENTINEL and _QT_SELECTION_SENTINEL but it
only hinted fellow Qt programs. GNOME Clipboard Manager used
polling to find out when the clipboard changes.
XFIXES adds the sorely-missing notification on selection
ownership change. That's when we re-query TARGETS and
re-announce them via RDP (as native CF_ clipboard formats,
as we sit fit). Without XFIXES library, or without it
available on the server at runtime, this patch falls back to
the old behavior of always announcing CF_(UNICODE)TEXT to
RDP and then refusing it at runtime if it's not available.
Yes, it makes our code a bit more hairy (e.g. see the
handling of the TARGETS target) but at least it gives the
best-available functionality.

> Third, both I and Peter Åstrand are very sceptical to the
removal of the PRIMARY support.

See my previous posts - I agree. However, I think we should
use _one_ of them, as using both adds a non-trivial and
unexpected behavior to the clipboard. It's better not to
have anything pasted than to have the clipboard "like a box
of chocolate" ("You never know what you are going to get"
:). Also, FreeDesktop-compliant applications which use
rdesktop as a backend would benefit from being able to make
it use PRIMARY, thus give their users a more-even experience.

> I'd just like to receive it in smaller pieces :-). 

I can do it. Are you sure it's a must, though? I think I now
understand the clipboard model (sans the INCR part, which I
intentionally didn't touch) better than anybody (but you :),
and I'd really want to move on to problematic areas, like
the non-standard-visuals-on-Irix bug or the Nvidia/ATI
visual corruption bug.

> Oh, btw, the patch in its current form doesn't compile due
to two incorrect lines in xclip.c when using DEBUG_CLIPBOARD
(lines 1045 and 1049).

Oops. I probably added this string after making my final
test-with-xfixes. So far, I tested the code with both XFIXES
and without, for both X11 clipboard data (conversion from
X11 clipboard formats <-> Windows native clipboard formats)
and RDP clipboard data (transferring drawings between two
rdesktop sessions running MS Paint).

----------------------------------------------------------------------

Comment By: Erik Forsberg (forsberg)
Date: 2005-11-04 15:26

Message:
Logged In: YES 
user_id=9251

Tried to apply this patch to the current cvs, compiled it
with iconv but without Xfixes, and tried to copy stuff from
winword on a w2003 to Openoffice 2.0 beta on a KDE 3.2 on
top of SLES9. Doesn't work. Output from clipboard debug:

CLIPRDR recv: type=2, status=0, length=396
CLIPRDR send: type=3, status=1, length=0
channel_send, length = 12
Sending 12 bytes with FLAG_FIRST
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=STRING, property=CLIPBOARD
CLIPRDR send: type=4, status=0, length=4
channel_send, length = 16
Sending 16 bytes with FLAG_FIRST
CLIPRDR recv: type=5, status=1, length=8
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=CLIPBOARD
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=COMPOUND_TEXT, property=_QT_SELECTION
Requested target unavailable. (It was not in TARGETS, so why
did you ask for it?!)
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=_QT_SELECTION
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=CLIPBOARD
xclip_handle_SelectionRequest: selection=CLIPBOARD,
target=TARGETS, property=CLIPBOARD

The opposite way (OO -> winword) works, though. 

Just something for you to think about.. :-)

----------------------------------------------------------------------

Comment By: Erik Forsberg (forsberg)
Date: 2005-11-04 14:42

Message:
Logged In: YES 
user_id=9251

As the original author of the clipboard functionality, I do
have some opinions on this patch.

First of all, the patch is too big. Understanding a 48k
patch is too much work, at least for my small brain :-). So,
as a first action, please split this patch into three
different patches. One that removes PRIMARY support, one
that adds XFixes support, and one that adds Unicode support.
Perhaps even a fourth part, if the patch by David Lowry is
incorporated into this gigantic patch?

Second, I'd like an explanation of what is gained by using
the XFixes extension. "provide cleaner support" as an
explanation is not enough to convince me to apply the patch.

Third, both I and Peter Åstrand are very sceptical to the
removal of the PRIMARY support. There are too many legacy
X11 applications that only understand PRIMARY. The ability
to cooperate with old X11 applications is a strength we
should keep. One example is such a common application as xterm. 

Your work is highly appreciated. I'd just like to receive it
in smaller pieces :-). 

Oh, btw, the patch in its current form doesn't compile due
to two incorrect lines in xclip.c when using DEBUG_CLIPBOARD
(lines 1045 and 1049).

----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-03 13:13

Message:
Logged In: YES 
user_id=335423

> My mum, who is hardly a hard-core sysadmin,
uses select to copy and middle click to paste, simply
because it's easier.

From my experience, a user who doesn't control the mouse
very well, frequently mis-selects text. He'd rather have a
stable clipboard. Against, I use PRIMARY often in my own use
of X, but this is a shortcut functionality, an advaced user
easter egg. CLIPBOARD is the more stable choice (cause its
controls are more explicit), but once you're stable enough
in your control of the computer, you may start using the
more-tricky time savers.

> It works whether or not the user uses middle-click, or
explicit Paste.

Cons of using both selections:
1. A user who learned, from his modern X apps, that
CLIPBOARD and PRIMARY are a different thing, would expect
one of them will be used all the time.
2. It creates a state of uncertainity, as we don't offer any
visual cue as to which selection will be used in the
forthcoming paste.
3. It creates an RDP mixup, since we won't know on which
selection's TARGETS list to base ourselves when announcing
our clipboard formats.

> It doesn't work, of course, if the user then goes and
selects something
else, but then the user clearly sees the mistake they made
when it pastes the wrong thing.

Q: Why the user pasted the wrong thing?
A: Because he didn't have a visual cue of whether PRIMARY is
full or empty now... and he'll be making this small mistake
throughout his entire day (you know, that frustrating "small
mistake"), cause he'll never be sure "Hmm, do I have
something selected in that minimized Mozilla window there?".

That is all assuming the order is:
1. PRIMARY
2. CLIPBOARD
-- which is the only order that makes sense, since most apps
don't have an explicit "Clear CLIPBOARD" function.

> The alternatives have the
problem that the user either ends up not pasting anything or
ends up with a latched selection

Not pasting anything is not a big deal if it happens just
once and then you get to learn which one of the selections
rdesktop (or "tsclient", or "krdc") uses.
What do you mean by "latched selection"?

> I don't see the need for more options, we have too many
options already.

From a user perspective, this is indeed one option too much.
However, you must remember that, as much as rdesktop is a
standalone application, we're also a backend for GUI Remote
Desktop clients. We're not a library -- we're a backend in
the old Unix
pipes-command-line-arguments-embed-into-X-window sense, but
still a backend. Since those FreeDesktop.org-compliant apps
cannot modify our clipboard model directly, we need to offer
them a way to influence it to be more compatible with their
own model.

----------------------------------------------------------------------

Comment By: Matt Chapman (matthewc)
Date: 2005-11-03 12:47

Message:
Logged In: YES 
user_id=60189

Yes, I read the URL.  I agree in principle that PRIMARY
should be used for select/middle-click, and CLIPBOARD should
be used for Edit->Copy/Paste.  However, it doesn't say
anything useful about a case like rdesktop, where the
application cannot distinguish the two.

It's not a question of hard-core sysadmins not wanting to
change, it's a question of preserving useful X
functionality.  My mum, who is hardly a hard-core sysadmin,
uses select to copy and middle click to paste, simply
because it's easier.  Why use extra keystrokes?  That's one
advantage that X has over Windows.

The current rdesktop behaviour, using PRIMARY and then
CLIPBOARD, allows it to work whichever method the user uses.
 It works whether or not the user uses middle-click, or
explicit Paste.  It works whether the user copies with
select, or explicit Copy.  It works even if the user
deselects the selection after doing the Copy.  It doesn't
work, of course, if the user then goes and selects something
else, but then the user clearly sees the mistake they made
when it pastes the wrong thing.  The alternatives have the
problem that the user either ends up not pasting anything or
ends up with a latched selection, both of which are
non-intuitive and will result in frustration.

I don't see the need for more options, we have too many
options already.


----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-03 12:27

Message:
Logged In: YES 
user_id=335423

Did you read the URL I've pasted? The "usual" ways are
changing and what those sysadmins are used to is no longer
right in Gtk2 apps, Qt3 apps, Mozilla... There's only a
handful of legacy X apps which still only use PRIMARY.

One thing is certain: Using both selections is throughly
confusing, as the user has no indication which of them will
be used.

Know what? Since Windows has only one selection and we have
two, there's no right and wrong here: I think we should
offer to use either of them, and allow the user to pick
*one* via a command line option. In desktop environments (as
opposed to a hardcore sysadmin type using twm), the user
will launch rdesktop via a GUI tool such as krdc or tsclient
anyway, and that GUI tool will instruct rdesktop to use the
CLIPBOARD buffer to fit better with modern FreeDesktop apps.

----------------------------------------------------------------------

Comment By: Matt Chapman (matthewc)
Date: 2005-11-03 12:19

Message:
Logged In: YES 
user_id=60189

I think it should check PRIMARY first.  If one was to check
CLIPBOARD first, then the selection would be "stuck" as soon
as you copied anything to the CLIPBOARD, and you could no
longer utilise PRIMARY, you'd have to launch xclipboard to
clear the clipboard.  PRIMARY on the other hand is easy to
clear, should you want rdesktop to use CLIPBOARD.

I just asked a couple of system administrators here and
they're in agreement that rdesktop should use PRIMARY, since
that's the "usual" way to cut and paste in X and creates the
least confusion.  However I don't object to using CLIPBOARD
as well.


----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-11-03 11:45

Message:
Logged In: YES 
user_id=335423

When rdesktop acquires clipboard ownership, it can take
ownership of both.
When rdesktop requests clipboard data from somebody else,
should it check PRIMARY first or CLIPBOARD first? Logics say
it should check CLIPBOARD first, since all modern (=
non-xterm) X11 apps follow the FreeDesktop.org model of
clipboard behavior, as described here:
http://www.freedesktop.org/wiki/Standards_2fClipboardsWiki

----------------------------------------------------------------------

Comment By: Matt Chapman (matthewc)
Date: 2005-11-03 11:38

Message:
Logged In: YES 
user_id=60189

The one thing I'm not comfortable with is dropping PRIMARY
support.  X users are familiar with using middle-click to
paste.  A lot of X applications don't have easy or obvious
facilities to paste from CLIPBOARD - e.g. xterm, how does
one paste CLIPBOARD and not PRIMARY into xterm?  I didn't
even know about the CLIPBOARD selection before I started
writing X apps.

The fact is that most applications do end up using both
PRIMARY and CLIPBOARD, because even if you're going to use
explicit copy and paste, you have to select first. 
Unfortunately rdesktop doesn't know when the user selects
something in Windows, but at the time that the user does the
explicit copy, it can update both.

----------------------------------------------------------------------

Comment By: Ilya Konstantinov (ikonst)
Date: 2005-10-28 17:07

Message:
Logged In: YES 
user_id=335423

In its XFixes variant, the patch isn't all correct. A new
patch will follow soon.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=381349&aid=1339819&group_id=24366


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
rdesktop-devel mailing list
rdesktop-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rdesktop-devel
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic