From kwin Tue Jan 24 22:04:50 2012 From: "Commit Hook" Date: Tue, 24 Jan 2012 22:04:50 +0000 To: kwin Subject: Re: Review Request: Fix indirect(?) wm_take_focus(?) handling Message-Id: <20120124220450.27939.4165 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kwin&m=132744293216679 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============8968477233131124907==" --===============8968477233131124907== Content-Type: multipart/alternative; boundary="===============2379627027595029341==" --===============2379627027595029341== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103700/#review10060 ----------------------------------------------------------- This review has been submitted with commit 9c377f223edb14d83ae079f203c36244= c3208a0e by Thomas L=C3=BCbking to branch master. - Commit Hook On Jan. 14, 2012, 8:48 p.m., Thomas L=C3=BCbking wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103700/ > ----------------------------------------------------------- > = > (Updated Jan. 14, 2012, 8:48 p.m.) > = > = > Review request for kwin. > = > = > Description > ------- > = > This one's weird. > If there's more than one plasma activity, activating a client (from kwin)= triggers changeActiveWindow() from a root window event. > One of the outcomes is the behavior described in the attached bug, but ac= tually activating a window becomes the same as > a) activating it > b) make the window call for self-activation > = > ----------------- > = > I'm not sure what exactly causes this (ie.. activity implementation in pl= asma-desktop or kwin), but what happens is: > = > activation.cpp > void Workspace::requestFocus(Client* c, bool force) > -> void Workspace::takeActivity(Client* c, int flags, bool handled) > -> client.cpp > void Client::takeActivity(int flags, bool handled, allowed_t) > -> void Client::takeFocus(allowed_t) > -> XSetInputFocus(display(), window(), RevertToPointerRoot, xTime()); > = > =3D=3D> events.cpp // from this point we handle the property change on th= e root window > void RootInfo::changeActiveWindow(Window w, NET::RequestSource src, Time = timestamp, Window active_window) > // workspace->activateClient(c); > -> activation.cpp > Workspace::activateClient(Client *c); > -> raise() // by the bugreport unwanted > = > ------------- > = > The patch fixes it by ignoring self-activating calls of the last client w= e intended to pass the focus anyway. > This fixes a bug, so it's suitable for 4.8, but it's very late and w/o re= ally knowing what's actually broken and given the present bug will affect a= minority (i dare to claim that the amount of pp. using such focus policy, = no autoraise and yet interested in activities is close to void.) i'd rather= suggest to not add it for 4.8.0 but test it locally and add it for 4.8.1 i= f there's no apparent side-effect. > Or we figure what's actually happening there to break it. > = > = > This addresses bug 240673. > http://bugs.kde.org/show_bug.cgi?id=3D240673 > = > = > Diffs > ----- > = > kwin/events.cpp 26e0139 = > = > Diff: http://git.reviewboard.kde.org/r/103700/diff/diff > = > = > Testing > ------- > = > Yes, adding a second activity and changing activities reliably caused thi= s for me (across WM restarts!) - it's gone with the patch (and why that wor= ks, i actually /do/ understand ;-) > = > = > Thanks, > = > Thomas L=C3=BCbking > = > --===============2379627027595029341== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/103700/ 

This revie=
w has been submitted with commit 9c377f223edb14d83ae079f203c36244c3208a0e b=
y Thomas L=C3=BCbking to branch master.

- Commit


On January 14th, 2012, 8:48 p.m., Thomas L=C3=BCbking wrote:

Review request for kwin.
By Thomas L=C3=BCbking.

Updated Jan. 14, 2012, 8:48 p.m.

Descripti= on 

This one's weird.
If there's more than one plasma activity, activating a client (from kwi=
n) triggers changeActiveWindow() from a root window event.
One of the outcomes is the behavior described in the attached bug, but actu=
ally activating a window becomes the same as
a) activating it
b) make the window call for self-activation

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

I'm not sure what exactly causes this (ie.. activity implementation in =
plasma-desktop or kwin), but what happens is:

activation.cpp
void Workspace::requestFocus(Client* c, bool force)
-> void Workspace::takeActivity(Client* c, int flags, bool handled)
-> client.cpp
void Client::takeActivity(int flags, bool handled, allowed_t)
-> void Client::takeFocus(allowed_t)
-> XSetInputFocus(display(), window(), RevertToPointerRoot, xTime());

=3D=3D> events.cpp // from this point we handle the property change on t=
he root window
void RootInfo::changeActiveWindow(Window w, NET::RequestSource src, Time ti=
mestamp, Window active_window)
// workspace->activateClient(c);
-> activation.cpp
Workspace::activateClient(Client *c);
-> raise() // by the bugreport unwanted

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

The patch fixes it by ignoring self-activating calls of the last client we =
intended to pass the focus anyway.
This fixes a bug, so it's suitable for 4.8, but it's very late and =
w/o really knowing what's actually broken and given the present bug wil=
l affect a minority (i dare to claim that the amount of pp. using such focu=
s policy, no autoraise and yet interested in activities is close to void.) =
i'd rather suggest to not add it for 4.8.0 but test it locally and add =
it for 4.8.1 if there's no apparent side-effect.
Or we figure what's actually happening there to break it.

Testing <= /h1> 
Yes, adding a second activity and changing activities reliab=
ly caused this for me (across WM restarts!) - it's gone with the patch =
(and why that works, i actually /do/ understand ;-)
Bugs: 240673

Diffs=

  • kwin/events.cpp (26e0139)

View Diff
--===============2379627027595029341==-- --===============8968477233131124907== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kwin mailing list kwin@kde.org https://mail.kde.org/mailman/listinfo/kwin --===============8968477233131124907==--