I would change the first label that says

"Action on Print key when Spectacle is running"

To something like:

When Spectacle is Running

Press Print Key to:

• Take New Screenshot
• Start a new Spectacle Window
• Return Focus to Spectacle

So like this?

When Spectacle is Running
       Press Print Key to: (o) Take a new Screenshot
                           ( ) Start a new Spectacle window
                           ( ) Return focus to Spectacle

(checkbox labels use sentence case, not title case)

Or did you envision "When Spectacle is Running" being like a header, as with the current "Rectangular Region" text?

Thanks Nate, my markup is awful.

I mean the second option where the "When Spectacle is Running" becomes a title.

Thanks for clarifying.

OK cool, let's give that a try. I think it could work nicely.

Awesome!

Nice! So now the only problem I can still find is that the "Return focus to Spectacle" option doesn't work, just like you said. I don't know enough about how Spectacle interacts with the window manager to help with that, but maybe @zzag or @davidedmundson can offer some insight here?

In D19153#415543, @ngraham wrote:

Nice! So now the only problem I can still find is that the "Return focus to Spectacle" option doesn't work, just like you said. I don't know enough about how Spectacle interacts with the window manager to help with that, but maybe @zzag or @davidedmundson can offer some insight here?

Also I still need to figure out why the setting doesn't get updated in the old istance when it is changed by a newer Instance.

Nevermind this does nothing if the window is minimized.

In D19153#415543, @ngraham wrote:

but maybe @zzag or @davidedmundson can offer some insight here?

Try different fsp policy. Though in general clients shouldn't mess with focus, fwiw on wayland it will most likely backfire on us.

In D19153#415952, @zzag wrote:

In D19153#415543, @ngraham wrote:

but maybe @zzag or @davidedmundson can offer some insight here?

Try different fsp policy. Though in general clients shouldn't mess with focus, fwiw on wayland it will most likely backfire on us.

Sorry, could you explain what fsp stands for?

I could guess we could use KWindowSystem::forceActiveWindow but the Documentations reads as @zzag said:

https://api.kde.org/frameworks/kwindowsystem/html/classKWindowSystem.html#aecb213d950a6615fc0e961273d72e166
Note that this should be called only in special cases, applications shouldn't force themselves or other windows to be the active window. Generally, this call should used only by pagers and similar tools. See the explanation in description of activateWindow().

And activateWindow() reads

There are two ways how to activate a window, by calling activateWindow() and forceActiveWindow(). Generally, applications shouldn't make attempts to explicitly activate their windows, and instead let the user to activate them. In the special cases where this may be needed, applications should use activateWindow(). Window manager may consider whether this request wouldn't result in focus stealing, which would be obtrusive, and may refuse the request.
The usage of forceActiveWindow() is meant only for pagers and similar tools, which represent direct user actions related to window manipulation. Except for rare cases, this request will be always honored, and normal applications are forbidden to use it.

However in our case we would call this only if configured by the user to do so and explicitly requested by the user by pressing the Print key. Also if spectacle isn't running and the Print key is pressed , currently Spectacle will open and steal the focus even if I'm typing in another application for example Konsole. Considering these two points I think it's reasonable to argue that this behavior would be acceptable.

In D19153#416016, @davidre wrote:

However in our case we would call this only if configured by the user to do so and explicitly requested by the user by pressing the Print key. Also if spectacle isn't running and the Print key is pressed , currently Spectacle will open and steal the focus even if I'm typing in another application for example Konsole. Considering these two points I think it's reasonable to argue that this behavior would be acceptable.

I would agree. The user is specifically requesting the behavior.

All right then I will update this tomorrow. Do you think we should also have a "None" option? Also which option should be the default one? I think either take a new screenshot or none (if added).

Yeah, I think Take a new screenshot makes sense as the default option.

Sorry, could you explain what fsp stands for?

Focus Stealing Prevention.

The usage of forceActiveWindow() is meant only for pagers and similar tools, which represent direct user actions related to window manipulation. Except for rare cases, this request will be always honored, and normal applications are forbidden to use it.

This clearly says that you should not use forceActiveWindow.

I urge you not to deal with focus. On X11, applications usually abuse such power, so it led to things like fsp. For what it's worth, clients can't activate by themselves on Wayland \o/.

In D19153#416515, @zzag wrote:

I urge you not to deal with focus. On X11, applications usually abuse such power, so it led to things like fsp. For what it's worth, clients can't activate by themselves on Wayland \o/.

So on Wayland, what is the way for an unfocused application to bring itself to the foreground and gain focus if requested to do so by the user? For that matter, what's the correct way to do this on X11?

In D19153#416675, @ngraham wrote:

So on Wayland, what is the way for an unfocused application to bring itself to the foreground and gain focus if requested to do so by the user?

As far as I know, there isn't. If I recall correctly, David started discussion on this matter some time ago on wayland-devel ml.

For that matter, what's the correct way to do this on X11?

In D19153#416016, @davidre wrote:

There are two ways how to activate a window, by calling activateWindow() and forceActiveWindow(). Generally, applications shouldn't make attempts to explicitly activate their windows, and instead let the user to activate them. In the special cases where this may be needed, applications should use activateWindow(). Window manager may consider whether this request wouldn't result in focus stealing, which would be obtrusive, and may refuse the request.

It will basically send a _NET_ACTIVE_WINDOW message to the root window, which indicates that the request comes from an application.

On Wayland we should just forget about the feature? That seems... less that ideal. What's the plan to fix this?

On X11, when we use activateWindow() the desired feature does not actually work with KWin's default activation/focus stealing/whatever settings: the Task Manager button turns orange, but the window doesn't come to the front. That's not useful. Considering that with this feature, the user is explicitly and deliberately setting a non-default setting that they believe should always result in Spectacle coming to the forefront no matter what, I think the use of forceActiveWindow() makes sense here. The user is in essence specifically overriding any prior focus-related preference they might have expressed elsewhere.

I'm willing to entertain more arguments about the most technically correct way to implement the "make spectacle come to the front" feature. However I will not accept "don't do that, it won't work." Because right now, it does work, and we have users who have specifically requested this feature. If the current implementation using forceActiveWindow() is problematic, then we need to make activateWindow() work the way we need it to work, or add some new API that does what we need.

On X11, when we use activateWindow() the desired feature does not actually work with KWin's default activation/focus stealing/whatever settings: the Task Manager button turns orange, but the window doesn't come to the front. That's not useful.

As expected. if a window attempted to become active but KWin prevented this, then it will be marked as demanding attention (it will be highlighted with orange color in the task manager).

Considering that with this feature, the user is explicitly and deliberately setting a non-default setting that they believe should always result in Spectacle coming to the forefront no matter what, I think the use of forceActiveWindow() makes sense here. The user is in essence specifically overriding any prior focus-related preference they might have expressed elsewhere.

I'm afraid that there is no pretty solution for this problem. It will be wrong if spectacle starts fooling kwin that the request comes from a pager. What would prevent other clients from doing that? They also act on behalf of user, they also need precious attention from user.

Generally speaking, the best solution would be just to avoid any attempts to activate some particular window. Given that Spectacle is out of scope of KWin developers, do whatever you want.

Ok, so let's do this for now @davidre:

• On X11, keep the current code
• On Wayland, don't show the "Return focus to Spectacle" option at all since it won't work there.

Will follow up on the issue with KWindowSystem::activateWindow() elsewhere.

How things should work in an ideal situation:

KGlobalaccel would call your app directly
This has a token hidden in the message before it's dispatched to your client.

KGlobalAccel calls QX11Info::setAppUserTime (all internally and magically before dispatching to your code))
When you call raise, kwin can now tie your app to the key event and give you focus. Without that it gets denied.

QWindow::activate() / KWindowSystem::activeWindow do work when they're used "correctly".

For wayland we still need an equivalent - it would probably be a breaking change in kglobalaccel protocol.
I still need to work on that.

You're not being called directly from kglobalaccel, and goes via khotkeys (booooo!) where this info gets lost.

forceWindowActive bypasses the WM. It shouldn't really be used.

My general rule is that you should only ever use it if you have no other feasible option and in response to an explicit user action.

You meet both those criteria, but it would definitely be worth looking at moving this to be activated via kglobalaccel directly.

In D19153#416773, @davidedmundson wrote:

My general rule is that you should only ever use it if you have no other feasible option and in response to an explicit user action.

You meet both those criteria, but it would definitely be worth looking at moving this to be activated via kglobalaccel directly.

Thanks David, that makes sense. So let's use forceActivate() for now and make a plan to port to KGlobalAccel soon so we can do all of this The Right Way™.

I have spectacle listed in my Application Launchers on the Shortcut KCM (via jump actions) and in khotkeys .

They both operate in different ways. One does dbus methods, the other does "spectacle -f"
That's quite confusing.

From what I can see this patch will only work when set up though the second mode.

In D19153#416786, @davidedmundson wrote:

I have spectacle listed in my Application Launchers on the Shortcut KCM (via jump actions) and in khotkeys .

They both operate in different ways. One does dbus methods, the other does "spectacle -f"
That's quite confusing.

From what I can see this patch will only work when set up though the second mode.

A you said thispatch works for the dbus method as the action is defined per default on Neon in "Custom shortcuts" (I think and I simply thoug that it was the default. I see what you mean, beause it just runs the command "spectacle" and we can't detect how the program was started.
Maybe the patch should be about what should happen if Spectacle is running and another Instance is starting in GuiMode. Then we could check D-Bus if another Spectacle is running or better call startAgent() because we don't know in which Mode the other Instance was started.

Best solution might be to change the Exec line in the Jump Actions to "qdbus org.kde.Spectacle /blah/blah blah.blah"
(see the spectacle .desktop file)

Then it's back to one code path.

Are you sure that we wan't these actions to hit the same codepath as StartAgent?

In D19153#417025, @davidedmundson wrote:

Best solution might be to change the Exec line in the Jump Actions to "qdbus org.kde.Spectacle /blah/blah blah.blah"
(see the spectacle .desktop file)

Then it's back to one code path.

I thinks that's resonable but I think it's not directly to this patch because it's another issue where a user is requesting directly the current running Spectacle to take a screenshot.
I will open another Diff for that after this one.

Excellent. Can you make the focus option not appear on Wayland? Then I think we'll be good to go.

Great! Now what happens if a user sets the focus option on X11, logs into a Wayland session, and tries to use it? :)

Looks great now! One final thing: can you see about reading the configuration to find the keyboard shortcut rather than always assuming it's PrintScreen?

Reading khotkeys would be difficult to do reliably.

I would suggest we find a generic term.

"Activation Key" ?

In D19153#417109, @ngraham wrote:

"Activation Key" ?

I think we would have to make it clear that it is the same Key as the user configured in the Custom Shortcut menu.

Honestly I think just using the term "PrintScreen key" is fine. It's accurate for the 99% common case where the user has not changed the shortcut, and any advanced user who has will understand what's meant by the string.

I'm okay with that.

Me too, but let's make sure @davidedmundson is too.

Hey, I like "screenshot key"! What do you think @davidre?

Yes that's better!