Fix on click not working
ClosedPublic

Authored by davidre on Jun 28 2019, 7:22 PM.

Details

Summary

When on click is enabled the timeout is always set to -1.
This breaks the logic inside captureScreenshot to display the progressbar in the
taskmanager.

BUG: 409218

Test Plan

Check on click or use spectacle in a wayland session

Diff Detail

Repository
R166 Spectacle
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Jun 28 2019, 7:22 PM
Restricted Application added a project: Spectacle. · View Herald TranscriptJun 28 2019, 7:22 PM
davidre requested review of this revision.Jun 28 2019, 7:22 PM
ngraham edited the summary of this revision. (Show Details)Jun 28 2019, 8:07 PM
anthonyfieroni added inline comments.
src/Gui/KSMainWindow.cpp
276–279

Just make it 0

if (theTimeout < 0) {
    theTimeout = 0;
}
davidre added inline comments.Jun 29 2019, 6:36 AM
src/Gui/KSMainWindow.cpp
279

I had the same idea but that unfortunately doesn't work because the timeout is used inside spectacle core to figure out if the screenshot should happen after a time or on click. If it isn't onclick platformkwinwayland will just return and do nothing

anthonyfieroni added inline comments.Jun 29 2019, 8:01 AM
src/Gui/KSMainWindow.cpp
279

To me, it still better to be there, you can discard time in platformwayland and just do it. You can wait for other opinions as well.

It works for me.

Why don't we support timeouts on Wayland? Is there anything fundamental blocking it?

It works for me.

Why don't we support timeouts on Wayland? Is there anything fundamental blocking it?

Because the available KWin functions that we use to read the image from a pipe on wayland show a confirmation overlay that the user has to click to take a screenshot or has to select with his click which screen/window to capture.

davidre added inline comments.Jul 3 2019, 9:03 AM
src/Gui/KSMainWindow.cpp
279

After thinking about it that wouldn't work because then we couldn't differentiate on X anymore, too.

Any more comments? I think we have to get this into 19.08

davidre retitled this revision from Fix Spectacle not working on Wayland to Fix on click not working.Jul 10 2019, 9:54 AM
davidre edited the summary of this revision. (Show Details)
davidre edited the test plan for this revision. (Show Details)
ngraham accepted this revision.Jul 10 2019, 7:17 PM
ngraham added a subscriber: ngraham.

I think this is fine. Let's get it into 19.08.0. Thanks a lot, @davidre!

This revision is now accepted and ready to land.Jul 10 2019, 7:17 PM

I think this is fine. Let's get it into 19.08.0. Thanks a lot, @davidre!

Well, when I break it, it is on me to fix it ;)

davidre updated this revision to Diff 61568.Jul 11 2019, 8:20 AM
  • Update comment
davidre updated this revision to Diff 61569.Jul 11 2019, 8:24 AM
  • Also hide when onClick is checked
This revision was automatically updated to reflect the committed changes.