Bug 422874

Summary: Spectacle shows a notification without screenshot preview when 'Copy image to clipboard' feature is enabled
Product: [Applications] Spectacle Reporter: Patrick Silva <bugseforuns>
Component: GeneralAssignee: Boudhayan Gupta <me>
Status: RESOLVED FIXED    
Severity: normal CC: akselmo, antonio.prcela, bharadwaj.raju777, dev.bacteriostat, kde, kortrax11, lehoangphuongbg, nate
Priority: NOR Keywords: regression
Version: 24.01.75   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: screenshot
notification on openSuse krypton
two notifications on neon unstable
Two notifications on Fedora KDE 37

Description Patrick Silva 2020-06-12 14:03:09 UTC
Created attachment 129261 [details]
screenshot

STEPS TO REPRODUCE
1. check "Copy image to clipboard" in Spectacle preferences
2. close Spectacle
3. take a screenshot by pressing shift+print

OBSERVED RESULT
notificaton without screenshot preview, see the attached screenshot please.

EXPECTED RESULT
notification should show screenshot preview regardless "Copy image to clipboard" feature is enabled or not.

SOFTWARE/OS VERSIONS
Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.19.80
KDE Frameworks Version: 5.71.0
Qt Version: 5.14.2
Comment 1 Nate Graham 2020-06-12 20:39:09 UTC
Cannot reproduce, works for me.
Comment 2 Patrick Silva 2020-06-17 21:05:43 UTC
Created attachment 129463 [details]
notification on openSuse krypton

Humm, I also can reproduce on openSuse krypton.

Operating System: openSUSE Tumbleweed 20200609
KDE Plasma Version: 5.19.80
KDE Frameworks Version: 5.71.0
Qt Version: 5.15.0
Comment 3 Antonio Prcela 2020-10-08 19:18:53 UTC
Can reproduce it, but atm I can't fix the piece of code :(

Operating System: Manjaro Linux
KDE Plasma Version: 5.19.5
KDE Frameworks Version: 5.74.0
Qt Version: 5.15.1
Kernel Version: 5.8.14-1-MANJARO
OS Type: 64-bit
Comment 4 Patrick Silva 2020-12-10 16:48:03 UTC
Created attachment 133974 [details]
two notifications on neon unstable

As we see in the attached screenshot, I currently get two notifications after the provided steps on neon unstable, one with screenshot preview and one without.

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.20.80
KDE Frameworks Version: 5.77.0
Qt Version: 5.15.2
Comment 5 Antonio Prcela 2020-12-10 18:45:09 UTC
(In reply to Patrick Silva from comment #4)
> Created attachment 133974 [details]
> two notifications on neon unstable
> 
> As we see in the attached screenshot, I currently get two notifications
> after the provided steps on neon unstable, one with screenshot preview and
> one without.
> 

It also seems to save the picture to the default location, even tho the Autosave is off. Do you also have that bug?
Comment 6 Patrick Silva 2020-12-10 19:44:06 UTC
(In reply to Antonio Prcela from comment #5)
> (In reply to Patrick Silva from comment #4)
> > Created attachment 133974 [details]
> > two notifications on neon unstable
> > 
> > As we see in the attached screenshot, I currently get two notifications
> > after the provided steps on neon unstable, one with screenshot preview and
> > one without.
> > 
> 
> It also seems to save the picture to the default location, even tho the
> Autosave is off. Do you also have that bug?

yes
Comment 7 Antonio Prcela 2021-01-06 16:10:00 UTC
atm I'm on spectacle 20.12.0 and when pressing Shift+Print with the "Copy image to clipboard" activated in settings, it only saves the image to my clipboard and only one Notification shows up.
So this brings us back to the original bug where there is no image in the notification :)
Comment 8 Bharadwaj Raju 2021-05-05 17:07:57 UTC
Currently KNotifications only supports image preview through a file URL, not through direct image in memory (it does support setting an image as icon). If Spectacle is set to copy to clipboard, and not to autosave anywhere, then there won't be any file URL. 

Possible solutions:
- save image to temporary directory, and pass that to Spectacle. potentially a problem because the notification would be in notification history so the "temporary" image would have to be kept for a rather long time.
- somehow change KNotifications and Plasma notification system to support having image previews from memory.
Comment 9 David Redondo 2021-05-07 07:22:39 UTC
It's possible to set a pixmap
Comment 10 Bharadwaj Raju 2021-05-07 07:31:04 UTC
(In reply to David Redondo from comment #9)
> It's possible to set a pixmap

https://api.kde.org/frameworks/knotifications/html/classKNotification.html

I interpreted the documentation of setPixmap to mean that it will be used for the icon, given the reference to setIconName as an alternative.

I'll test if it really uses it as a preview and if so I'll make an MR.
Comment 11 David Redondo 2021-05-07 07:32:04 UTC
Oh then nevermind
Comment 12 K Freed 2022-01-24 15:22:29 UTC
Fixed the title to generalize the issue to all users here. Bug 449057 shows only the duplicate notifications issue.
Comment 13 K Freed 2022-01-24 15:30:36 UTC
(In reply to Bharadwaj Raju from comment #8)
> Currently KNotifications only supports image preview through a file URL, not
> through direct image in memory (it does support setting an image as icon).
> If Spectacle is set to copy to clipboard, and not to autosave anywhere, then
> there won't be any file URL. 
> 
> Possible solutions:
> - save image to temporary directory, and pass that to Spectacle. potentially
> a problem because the notification would be in notification history so the
> "temporary" image would have to be kept for a rather long time.
> - somehow change KNotifications and Plasma notification system to support
> having image previews from memory.

I don’t see solution 1 as a problem. Especially if the image is saved to a directory like /tmp/spectacle/notif-imgs. The directory would be cleared on reboot anyway.  Plus other apps do this.
The notif would say at this point “Image screenshot-xx-…. saved to clipboard.”
Comment 14 Patrick Silva 2022-05-23 15:04:42 UTC
STEPS TO REPRODUCE
1. uncheck 'Save file to default folder' and select 'Copy image to clipboard' in Spectacle preferences and click on 'OK' button
2. close Spectacle
3. take a screenshot by pressing shift+print

Result: notification without screenshot preview

If 'Save file to default folder' is checked and 'Copy image to clipboard' is selected, Spectacle shows two notifications as mentioned
in comment 4.

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.25.80
KDE Frameworks Version: 5.95.0
Qt Version: 5.15.4
Graphics Platform: Wayland
Comment 15 Bacteria 2022-05-24 09:08:37 UTC
I followed the instructions in comment 14 but I am not able to get the notification. I am not able to get notification with any possible setting.

Operating System: Arch Linux
KDE Plasma Version: 5.24.90
KDE Frameworks Version: 5.94.0
Qt Version: 5.15.4
Spectacle Version: 22.04.1
Graphics Platform: Wayland
Comment 16 Lê Hoàng Phương 2022-10-28 08:43:38 UTC
Currently, the "Copy to Clipboard" notification doesn't allow "Annotate". If "Annotate" feature needs the screenshot to be saved somewhere (my assumption only, I didn't read the code), having Spectacle to also save the screenshot to /tmp should allow "Copy to Clipboard" notification to both show preview, and "Annotate"-able.

I currently have to bear the duplicate notifications. I use the "rectangular region capture" shortcut day-to-day in my work. Most of the time I simply need to paste the shortcut to other document, chat screen, etc, for other times I also need annotate the screenshot before pasting it. Since the notification that have "Annotate" only appear with option "Save to default folder", there are now two notifications every time I make a screenshot.
Comment 17 Akseli Lahtinen 2023-02-26 20:27:55 UTC
I get two notifications on Fedora KDE. This began to happen when i set a sound effect for the screenshot notification in spectacles notification settings.

Spectacle 22.12.2

Operating System: Fedora Linux 37
KDE Plasma Version: 5.27.1
KDE Frameworks Version: 5.103.0
Qt Version: 5.15.8
Kernel Version: 6.1.13-200.fc37.x86_64 (64-bit)
Graphics Platform: Wayland
Processors: 12 × AMD Ryzen 5 3600 6-Core Processor
Memory: 15,5 GiB of RAM
Graphics Processor: AMD Radeon RX 6600
Comment 18 Akseli Lahtinen 2023-02-26 20:28:18 UTC
Created attachment 156765 [details]
Two notifications on Fedora KDE 37
Comment 19 Noah Davis 2023-04-10 16:09:49 UTC
Git commit 84a9d9762e168d1f505736895e326f4546db29c8 by Noah Davis.
Committed on 10/04/2023 at 16:02.
Pushed by ndavis into branch 'master'.

Use export action flags instead of several functions and signals

This allows us to specify export actions and figure out which kinds of
actions have been done simultaneously without having separate functions
and signals for all the various combinations of save, save as, copy
image and copy path.

SpectacleCore: Instead of storing which actions to use in boolean member
variables, use a function to calculate which automatic export options
can be done. This makes it so we don't need to update the variables
every time we want to use them.

CaptureWindow, SelectionEditor, SpectacleCore: Don't export directly
from CaptureWindows. Instead, pass export actions to the
SpectacleCore::grabDone() signal via SelectionEditor::acceptSelection().
Necessary for preventing double saves when clicking the Save button in
rectangle capture windows.

In order to cherry-pick this to 23.04, we'll need to cherry-pick some of
the previous code clean-up and bug fix commits on master:

- 35f3eefc2125aadd302519bee629fa380e0d1cce
- 0bf9cd3433e418ce15337490cf119e0e30f87ee6
- 6711242f8397417ae05eaf76dc5f81622b691473
- 3a9f5351deb887eeac5d751d44fa55e8c31e63d4
- a95993c25059ab8de85f7128ec87b32f6c7df9b4
- ed981b997cfc0d155db29af4bf7f04659903092e
- 28f63cee4fb71de2804994b582b084db77b2cfcc
- 211127024a500da6f9ccf27c1131682a5558eec0
- 553eb2cf16ad3ccb8f9177cfd0ec533de5f22fdc
- 0fda35cb61b8db8cc50e05e28ee33e7516ca7cd2
- 001877540c54c19ba0a08b34182744d285dac3bc
- abf044557f36115c8e1ace0ba86a5c020422288f

M  +82   -97   src/ExportManager.cpp
M  +15   -10   src/ExportManager.h
M  +4    -8    src/Gui/CaptureWindow.cpp
M  +2    -2    src/Gui/ExportMenu.cpp
M  +5    -5    src/Gui/SelectionEditor.cpp
M  +2    -1    src/Gui/SelectionEditor.h
M  +5    -23   src/Gui/SpectacleWindow.cpp
M  +4    -2    src/Main.cpp
M  +97   -116  src/SpectacleCore.cpp
M  +5    -5    src/SpectacleCore.h

https://invent.kde.org/graphics/spectacle/commit/84a9d9762e168d1f505736895e326f4546db29c8
Comment 20 Noah Davis 2023-04-10 16:14:32 UTC
Git commit 8e44d9d40a3e13b7197732624e7f566e6712c463 by Noah Davis.
Committed on 10/04/2023 at 16:14.
Pushed by ndavis into branch 'release/23.04'.

Use export action flags instead of several functions and signals

This allows us to specify export actions and figure out which kinds of
actions have been done simultaneously without having separate functions
and signals for all the various combinations of save, save as, copy
image and copy path.

SpectacleCore: Instead of storing which actions to use in boolean member
variables, use a function to calculate which automatic export options
can be done. This makes it so we don't need to update the variables
every time we want to use them.

CaptureWindow, SelectionEditor, SpectacleCore: Don't export directly
from CaptureWindows. Instead, pass export actions to the
SpectacleCore::grabDone() signal via SelectionEditor::acceptSelection().
Necessary for preventing double saves when clicking the Save button in
rectangle capture windows.

In order to cherry-pick this to 23.04, we'll need to cherry-pick some of
the previous code clean-up and bug fix commits on master:

- 35f3eefc2125aadd302519bee629fa380e0d1cce
- 0bf9cd3433e418ce15337490cf119e0e30f87ee6
- 6711242f8397417ae05eaf76dc5f81622b691473
- 3a9f5351deb887eeac5d751d44fa55e8c31e63d4
- a95993c25059ab8de85f7128ec87b32f6c7df9b4
- ed981b997cfc0d155db29af4bf7f04659903092e
- 28f63cee4fb71de2804994b582b084db77b2cfcc
- 211127024a500da6f9ccf27c1131682a5558eec0
- 553eb2cf16ad3ccb8f9177cfd0ec533de5f22fdc
- 0fda35cb61b8db8cc50e05e28ee33e7516ca7cd2
- 001877540c54c19ba0a08b34182744d285dac3bc
- abf044557f36115c8e1ace0ba86a5c020422288f


(cherry picked from commit 84a9d9762e168d1f505736895e326f4546db29c8)

M  +82   -97   src/ExportManager.cpp
M  +15   -10   src/ExportManager.h
M  +4    -8    src/Gui/CaptureWindow.cpp
M  +2    -2    src/Gui/ExportMenu.cpp
M  +5    -5    src/Gui/SelectionEditor.cpp
M  +2    -1    src/Gui/SelectionEditor.h
M  +5    -23   src/Gui/SpectacleWindow.cpp
M  +4    -2    src/Main.cpp
M  +97   -116  src/SpectacleCore.cpp
M  +5    -5    src/SpectacleCore.h

https://invent.kde.org/graphics/spectacle/commit/8e44d9d40a3e13b7197732624e7f566e6712c463
Comment 21 Patrick Silva 2023-05-12 15:04:19 UTC
Cannot reproduce two notifications at the same time, but the notification without a preview persists.

Operating System: Arch Linux 
KDE Plasma Version: 5.27.5
KDE Frameworks Version: 5.105.0
Qt Version: 5.15.9
Graphics Platform: Wayland
Comment 22 Patrick Silva 2023-11-19 16:51:23 UTC
Can reproduce a notification without preview with steps from comment 14.

Operating System: Arch Linux 
KDE Plasma Version: 5.27.80
KDE Frameworks Version: 5.245.0
Qt Version: 6.6.0
Graphics Platform: Wayland
Comment 23 Bug Janitor Service 2023-11-22 12:07:14 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/spectacle/-/merge_requests/292
Comment 24 Noah Davis 2023-11-22 21:24:58 UTC
Git commit e73b6d98542388db6c12c9f70145bb7abe56e5a0 by Noah Davis.
Committed on 22/11/2023 at 22:24.
Pushed by ndavis into branch 'master'.

Save image as temp file, then copy data when copying image

Now you can see a thumbnail of the image you copied in a notification
when only copying an image. The image is in whatever format you just
saved (if saving and copying image) or the default image format.

There is a known minor flaw where if you click the image thumbnail to open it in an image viewer, the image will be deleted before it can be viewed. This is because the image is in the temp dir, so it gets deleted when spectacle is closed.
Related: bug 465781

M  +21   -1    src/ExportManager.cpp

https://invent.kde.org/graphics/spectacle/-/commit/e73b6d98542388db6c12c9f70145bb7abe56e5a0