Review Request: Fix KWindowSystem::compositingChanged signal

[Thomas Lübking]

> On Jan. 8, 2013, 11:15 p.m., Xuetian Weng wrote:
> > after I apply the patch, I notice that in some random unknnow case KWindowSystem will emit the signal, while KSelectionOwner is not. (And I think KSelectionOwner is correct since I didn't do anything to kwin), and I notice some strange plasma theme change caused by this.
> > 
> > Trying to find you why..
> 
> Thomas Lübking wrote:
>     The XFixesSelectionNotify event fires one every selection owner event, usually it'll be the cnp system (when you eg. mark text)
>     
>     The problem should ideally be fixed in Qt instead of working around it in KDE.
>     Untested patch:
>     
>     
>     diff --git a/src/gui/kernel/qapplication_x11.cpp b/src/gui/kernel/qapplication_x11.cpp
>     index 3631f1d..644f0ad 100644
>     --- a/src/gui/kernel/qapplication_x11.cpp
>     +++ b/src/gui/kernel/qapplication_x11.cpp
>     @@ -536,6 +536,7 @@ struct qt_xfixes_selection_event_data
>      {
>          // which selection to filter out.
>          Atom selection;
>     +    Window window;
>      };
>      
>      #if defined(Q_C_CALLBACKS)
>     @@ -548,7 +549,7 @@ static Bool qt_xfixes_scanner(Display*, XEvent *event, XPointer arg)
>              reinterpret_cast<qt_xfixes_selection_event_data*>(arg);
>          if (event->type == X11->xfixes_eventbase + XFixesSelectionNotify) {
>              XFixesSelectionNotifyEvent *xfixes_event = reinterpret_cast<XFixesSelectionNotifyEvent*>(event);
>     -        if (xfixes_event->selection == data->selection)
>     +        if (xfixes_event->selection == data->selection && xfixes_event->window == data->window)
>                  return true;
>          }
>          return false;
>     @@ -3462,6 +3463,7 @@ int QApplication::x11ProcessEvent(XEvent* event)
>              // we don't want to handle old SelectionNotify events.
>              qt_xfixes_selection_event_data xfixes_event;
>              xfixes_event.selection = req->selection;
>     +        xfixes_event.window = req->window;
>              for (XEvent ev;;) {
>                  if (!XCheckIfEvent(X11->display, &ev, &qt_xfixes_scanner, (XPointer)&xfixes_event))
>                      break;
>
> 
> Xuetian Weng wrote:
>     but I don't see in near future there will be Qt 4.8.5 nor distro can pick up this patch..
>     
>     actually this can cause serious problem, clipboard is so widely used.
>     
>     I guess if would be better to workaround it for now, I already notice some strange theme change, since clipboard is being initialized by KApplication by default..
> 
> Xuetian Weng wrote:
>     And thanks god we have klipper by default... If not, plasma will simply go crazy because of no workaround...
>     copy text -> close -> plasma theme changed to opaque

Ah, ok - in case that's not clear:

                            THE PATCH AS IT IS NOW HERE IS ***NOT*** SAFE FOR DISTRIBUTION!

What is does is to proof that we get the event on the root event (aside the clipboard)

If we need a workaround in kdelibs, i'll write one (you can only rely on the trigger in that something *might* have just changed, but must verify the current state) but i was mostly calling for action (i had asked on the bug whether anyone could still reproduce it except for me) and information (esp. because of the "weird" kselectionowner behavior, which however only affects kwin and maybe some others with an extra X11 event filter)


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107983/#review25012
-----------------------------------------------------------


On Jan. 5, 2013, 5:08 p.m., Thomas Lübking wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107983/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2013, 5:08 p.m.)
> 
> 
> Review request for kdelibs, kwin, Plasma, Aaron J. Seigo, Marco Martin, Martin Gräßlin, and Fredrik Höglund.
> 
> 
> Description
> -------
> 
> It works fine here (tested so far KWindowSystem signal, KSelectionWatcher only with kwin) with kwin (shift+alt+f12), xcompmgr, compiz & "metacity -c" and e17.
> Didn't try xfce nor mutter.
> 
> Technically:
> I do not at all understand why KWindowSystem is *not* watching the root window - KSelectionOwner for one is sending events to the root and this also seems the case for all other WMs (at least everything now starts to cause the signal to be emitted)
> 
> The KSelectionWatcher failure seems to be kwin specific (wrote me a cleaner testcase), there'll be some X11 event processing on top that eats away the client messages.
> So this one can be scratched from the patch, the KWindowSystem issue remains.
> 
> 
> This addresses bug 179042.
>     http://bugs.kde.org/show_bug.cgi?id=179042
> 
> 
> Diffs
> -----
> 
>   kdeui/windowmanagement/kwindowsystem_x11.cpp f9b3cc1 
> 
> Diff: http://git.reviewboard.kde.org/r/107983/diff/
> 
> 
> Testing
> -------
> 
> see summary
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130109/78a1086c/attachment.html>