Add Screenshot portal
ClosedPublic

Authored by jgrulich on Apr 9 2018, 2:17 PM.

Details

Summary

Add Screenshot portal using KWin Screenshot DBus interface.

Diff Detail

Repository
R838 Flatpak Support: KDE Portal for XDG Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
jgrulich requested review of this revision.Apr 9 2018, 2:17 PM
jgrulich created this revision.
jgrulich added a subscriber: Plasma.
jgrulich updated this revision to Diff 31746.Apr 9 2018, 4:00 PM

Improve dialog layout

anthonyfieroni added inline comments.
src/screenshot.cpp
63–65

You call 3 times deleteLater (even 1 more than as needed) so you can simplified logic

QPointer<ScreenshotDialog> screenshotDialog = new ScreenshotDialog();
QImage screenshot = screenshotDialog->exec() ? screenshotDialog->image() : QImage{};
if (screenshotDialog) {
    screenshotDialog->deleteLater();
}

And dialog can be deleted in exec which will result in crash at line 81

Screenshot of the dialog:

jgrulich updated this revision to Diff 31748.Apr 9 2018, 4:10 PM

Simplify logic

jgrulich updated this revision to Diff 33147.Apr 26 2018, 1:05 PM

Make sure the file url we return is in correct format

Kanedias accepted this revision.Apr 29 2018, 11:04 PM

Looks ok to me, only minor question about how this works

src/screenshotdialog.cpp
129

Are we safe closing the fd 1 here? Are we sure it already got through asyncCall?

This revision is now accepted and ready to land.Apr 29 2018, 11:04 PM

It should be correct, because from Qt documentation, using QDbusUnixFileDescriptor creates a duplicate of that file descriptor so it should be safe to close it right after.

This revision was automatically updated to reflect the committed changes.