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

List:       kde-commits
Subject:    Re: kdebase/klipper
From:       Esben Mose Hansen <kde () mosehansen ! dk>
Date:       2005-02-07 15:55:59
Message-ID: 200502071655.08284.kde () mosehansen ! dk
[Download RAW message or body]

On Monday 07 February 2005 13:07, Lubos Lunak wrote:
> On Friday 04 of February 2005 21:25, Esben Mose Hansen wrote:
> > CVS commit by esben:
> >
> > This is a workaround for a (x)emacs bug.  The problem is that XEmacs
> > freezes if asked to copy >=262041 bytes. Thats just under 256, so I
> > suspect a char[256*1024] inside XEmacs.
>
>  I don't see anything in the patch that would be related to this.

See my explanation below. With the patch, Klipper freezes for 5s --- 
regrettable, but there is nothing to do about it that I can see. Without the 
patch, klipper freezes completely until the selection is removed.

>
> > To make matters worse, XEmacs
> > always respond none to the XConvertSelection with
> > Atom("TIMESTAMP"). Which causes Klipper to poll XEmacs every 1000ms,
> > each time causing QT to wait for 5s before timing out. In other words,
> > Klipper is out for the count until the user presses ctrl-G This fix
> > request the TARGETS property instead, which seem better supported.
>
>  Please revert. You cannot simply use another property just because it
> seems to work, just like you cannot use a different variable when something
> is wrong with the one you should use.

I'll revert when I have a better alternative. And you are painting a wrong 
image: I'm requesting some extra information; the time stamp is always 
included in the response. The request is  carefully chosen to maximize the 
chance to get a response while still not requesting too much information. 

I'm not terrible anxious to introduce bugs  just use a request type that saves 
a few bytes. 

I could agree to make Klipper retry with the TARGETS property if the TIMESTAMP 
returns NULL. I'll even try to make a patch to this effect if this is 
acceptable.

>
>  The TIMESTAMP target means the timestamp of when the selection has been
> acquired by the owner (http://tronche.com/gui/x/icccm/sec-2.html#s-2.6.2),
> and Klipper checks it to see whether the clipboard contents have changed.
> When XEmacs responds None to this target it means it doesn't support it, in
> which case Klipper simply has to poll the contents (like it's been doing
> since ever). There should be no problem with this[*].

But the original patch (not mine) did not make Klipper revert to polling. On 
the contrary it made Klipper believe that there was a new clipboard contents 
every polling interval. There was a number of bug reports that ultimately was 
due to this problem.

Klipper should not assume that all application support the TIMESTAMP target.

>
>  If there's something causing QClipboard timeouts, it's most probably
> something else. Do you have a specific bugreport for this problem?

It wasn't causing timeouts. It was causing constant "clipboard changed" event 
in Klipper. 

>
> [*] Well, actually Klipper should finally start using the XFixes extension
> for this when available. I wonder if a patch for this could count as a
> bugfix ;).

Let's do that for kde4, but we will probably have to support the current way 
for a time yet.


-- 
regards. Esben
[prev in list] [next in list] [prev in thread] [next in thread]