korgac: For no grab avoid mouse cursor when placing window
Needs ReviewPublic

Authored by dfries on Oct 16 2018, 4:18 AM.

Details

Reviewers
mlaurent
dvratil
Summary

Even if the window doesn't grab focus with activate window, placing
the window under the mouse cursor can will still cause it to get focus
(depending on the window manager and settings). If no grab is
selected, shift it to not be under the mouse.

Test Plan

With the dock window "Reminder grabs focus" unchecked, position the Reminders window/mouse to be at the same location, dismiss the Reminders window, show the Reminders window, verify the Reminders window shows not under the mouse.

Diff Detail

Repository
R226 Konqueror
Lint
Lint Skipped
Unit
Unit Tests Skipped
dfries created this revision.Oct 16 2018, 4:18 AM
Restricted Application added a project: KDE PIM. · View Herald TranscriptOct 16 2018, 4:18 AM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
dfries requested review of this revision.Oct 16 2018, 4:18 AM
dvratil requested changes to this revision.Oct 16 2018, 8:44 AM
dvratil added a subscriber: dvratil.

I don't know if this is a good thing or not - there's still the window decoration and mouse pointer size that you would need to take into account, so just moving the window arbitrary amount of pixels left/right/above/below the cursor may or may not be enough based on user's window decorations theme, cursor size and DPI settings.

Isn't there maybe some option in KWindowSystem that would allow us to tell to the window manager "don't give auto focus to this window at any cost"?

Coding style: space after if and for, opening curly braces on the same line as the if/for statement

korgac/alarmdialog.cpp
631

const bool

This revision now requires changes to proceed.Oct 16 2018, 8:44 AM

KWindowSystem::setUserTime(winId(), 0);
"The most common case is the special value 0 which means not to activate the window after being shown." Whatever that means, because I called it before QDialog::show(); (the documentation doesn't say and I'm assuming if you've already called show() it's too late), and didn't see a getting focus behavior difference in fvwm or kde (tested without this patch, so the Reminder window appeared where it was last).

// KDE with "Focus Follows Mouse - Mouse Precedence"
// called with 0, mouse not under window, gets focus
// not called, mouse not under window, gets focus
// KDE with "Click to Focus"
// called with 0, mouse not under window, doesn't get focus
// not called, mouse not under window, doesn't get focus
// called with 0, mouse under window, gets focus
// not called, mouse under window, gets focus

"there's still the window decoration and mouse pointer size that you would need to take into account"

+ // need to include the frame which can be under the cursor
+ QRect withFrame = frameGeometry();

That is to take care of the window decorations.
I don't see the mouse pointer size mattering, just the single pixel hot point.

Opinions? Will it be accepted if I fix up the patch, or do I just drop it? Most of the time I put the Reminder in the corner and so with the rest of the patches it doesn't get triggered.

I will need someone to push them for me once the sequence has been approved, I'll fixup D16245 tonight.

One other bug unrelated to this patch sequence and I wouldn't think it would be specific to korgac. If I don't set the theme it doesn't get stuck printing the message, but it doesn't have the theme. I didn't know if you would have any suggestions off the top of your head, other than start debugging.

Debian bug #910555

That bug was against libkf5notifications5 5.28, but 5.49 has the same problem.

With fvwm and stalonetray,
export QT_QPA_PLATFORMTHEME=kde
results in the following being printed over and over again and it's recursive, so I expect it to eventually bomb out.
QSystemTrayIcon::setVisible: No Icon set

I have /usr/share/korgac/icons/hicolor/32x32/apps/korgac.png
strace shows it gets to the directory, but doesn't load the file.
strace -e file korgac 2>&1 |grep korgac.icons.hicolor.32x32.apps
stat("/usr/share/korgac/icons/hicolor/32x32/apps", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0

dfries updated this revision to Diff 43834.Oct 18 2018, 3:59 AM
dfries marked an inline comment as done.

added const, edited if, for { style

@mlaurent what do you think about this?