Summary: | Default focus stealing prevention behavior has become too strict | ||
---|---|---|---|
Product: | [Plasma] kwin | Reporter: | Eike Hein <hein> |
Component: | core | Assignee: | KWin default assignee <kwin-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | normal | Flags: | mgraesslin:
ReviewRequest+
|
Priority: | NOR | ||
Version: | git master | ||
Target Milestone: | 5 | ||
Platform: | unspecified | ||
OS: | Linux | ||
URL: | https://git.reviewboard.kde.org/r/118456 | ||
Latest Commit: | http://commits.kde.org/kwin/7910fed659b2b0c5d2d7c15880b290fd444e7135 | Version Fixed In: | |
Sentry Crash Report: |
Description
Eike Hein
2014-06-01 04:17:27 UTC
"change" is due to lazy xTime() update by 99% chance - or the launcher getting into the focus chain (thus has the newer usertime by the mouserelease) Also see: https://git.reviewboard.kde.org/r/110919/ should go along https://git.reviewboard.kde.org/r/110918/ based on this thoughts http://forum.kde.org/viewtopic.php?f=83&t=101833 WARNING: the patches will not apply cleanly on master (but still should on 4.11) Can we accelerate progress on this somehow? I think this is fairly important to fix for the release; the current behavior is a huge nuisance. As for this bug, calling "updateXTime();" before calling xTime() in updateUserTime() in activation.cpp should be sufficient. Only question is whether we should still tackle this more generally. See bug #335187 Proposal there is to flag the timestamp dirty at the end of each eventcycle and have inline KWIN_EXPORT xcb_timestamp_t xTime() { if (m_timeStampDirty) { xcb_timestamp_t time = QX11Info::getTimestamp(); QX11Info::setAppTime(time); return time; } return QX11Info::appTime(); } then scratch updateXTime() and seek to replace xTime() by XCB_CURRENT_TIME wherever suitable . I can however not say whether this can/should be done before the P/N release. We've atm 33 lines of xTime() (quite some are already comments) and 13 calls to updateXTime() (In reply to comment #2) > Can we accelerate progress on this somehow? I think this is fairly important > to fix for the release; the current behavior is a huge nuisance. hey, it's Sunday and you opened that bug just a few hours ago. A little bit realistic time handling wouldn't hurt :-) I sent a reply which somewhere got lost: are you running https://git.reviewboard.kde.org/r/118234/ ? If not, could you please try whether that improves the situation. > hey, it's Sunday and you opened that bug just a few hours ago. A little bit realistic time handling wouldn't hurt :-) For the record, my "can we accelerate" was because Thomas' review requests he pointed to as fixes are 11 months old, and their description says "needs a lot of testing" -- I didn't mean in the context of today. > are you running https://git.reviewboard.kde.org/r/118234/ Nope - testing in a moment. > If not, could you please try whether that improves the situation.
Yep, seems to fix it! In the same situation, with focus stealing prevention set back to Low, Konsole is now raised and focused.
(In reply to comment #3) > I can however not say whether this can/should be done before the P/N release. > We've atm 33 lines of xTime() (quite some are already comments) and 13 calls > to updateXTime() sounds like post-release to me. I think there are quite some which can be removed. E.g. I added one updateXTime() in TabBox when activated through globalaccel, but that was in fact just a workaround for kglobalaccel not setting the proper timestamp as it used to be. Also I think that it should be possible to remove quite some updateXTime if we properly update the timestamp from the events we get (and check where it's done in Qt). I may have been too early with the "yes, it's fixed" above - right now, still in the same session with the same settings, launching Konsole from the panel no longer raises and focuses it. I don't know at what point or why the behavior changed. On Sunday 01 June 2014 13:40:42 you wrote:
> I may have been too early with the "yes, it's fixed" above - right now,
> still in the same session with the same settings, launching Konsole from
> the panel no longer raises and focuses it. I don't know at what point or
> why the behavior changed.
are we talking about Konsole/4 or Konsole/5 here? It's quite possible that
this is a regression in Konsole (after all it's still a unique application)
Konsole/5, but I'm aware of its single-process mechanic and it's a fresh process in all the test cases. The same also happens with Kate, though. diff --git a/activation.cpp b/activation.cpp index d98c7d9..ca194da 100644 --- a/activation.cpp +++ b/activation.cpp @@ -661,8 +661,10 @@ void Workspace::clientAttentionChanged(Client* c, bool set) void Client::updateUserTime(xcb_timestamp_t time) { // copied in Group::updateUserTime - if (time == XCB_TIME_CURRENT_TIME) + if (time == XCB_TIME_CURRENT_TIME) { + updateXTime(); time = xTime(); + } if (time != -1U && (m_userTime == XCB_TIME_CURRENT_TIME || NET::timestampCompare(time, m_userTime) > 0)) { // time > user_time in addition also: diff --git a/events.cpp b/events.cpp index e7185e0..d2d5819 100644 --- a/events.cpp +++ b/events.cpp @@ -606,12 +606,12 @@ bool Client::windowEvent(xcb_generic_event_t *e) propertyNotifyEvent(reinterpret_cast<xcb_property_notify_event_t*>(e)); break; case XCB_KEY_PRESS: - updateUserTime(); + updateUserTime(reinterpret_cast<xcb_key_press_event_t*>(e)->time); workspace()->setWasUserInteraction(); break; case XCB_BUTTON_PRESS: { const auto *event = reinterpret_cast<xcb_button_press_event_t*>(e); - updateUserTime(); + updateUserTime(event->time); workspace()->setWasUserInteraction(); buttonPressEvent(event->event, event->detail, event->state, event->event_x, event->event_y, event->root_x, event->root_y); @Eike: please give a try to the patch in https://git.reviewboard.kde.org/r/118456 I'm running it now - so far it seems good, but I'll need to use it for a while again. I've been running both patches all day and so far it seems to all behave nicely. Git commit 7910fed659b2b0c5d2d7c15880b290fd444e7135 by Martin Gräßlin. Committed on 01/06/2014 at 15:57. Pushed by graesslin into branch 'master'. Improve updating user timestamp Use the timestamp from the xcb event which triggers the update whenever possible. If we don't have access to the latest event, let's at least update our own xTime prior to using it. Slightly unrelated change included: Group switches the userTime from XLib datatype to xcb datatype. REVIEW: 118456 M +8 -4 activation.cpp M +2 -2 client.h M +10 -9 events.cpp M +4 -4 group.h http://commits.kde.org/kwin/7910fed659b2b0c5d2d7c15880b290fd444e7135 |