From: SourceForge.net <no...@so...> - 2005-11-07 12:26:05
|
Patches item #1339819, was opened at 2005-10-27 20:46 Message generated for change (Comment added) made by forsberg 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: Erik Forsberg (forsberg) Date: 2005-11-07 13: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 17: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 16: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 15: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 14: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 14: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 13: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 12: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 11: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 11: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 11: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 10: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 10: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 |