Bug 458955 - Pressing Esc silently closes annotation editor
Summary: Pressing Esc silently closes annotation editor
Status: RESOLVED UPSTREAM
Alias: None
Product: gwenview
Classification: Applications
Component: general (other bugs)
Version First Reported In: 22.08.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Gwenview Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-10 13:12 UTC by gudvinr+kde
Modified: 2022-09-15 16:28 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description gudvinr+kde 2022-09-10 13:12:09 UTC
SUMMARY

When you press Esc in annotation editor and nothing is active (e.g. text input) it closes without warning which leads to losing data. I actually learned that hard way by just pressing Esc twice to deactivate text input.
That's rookie mistake in terms of usability.

I am not sure if that's right place for this issue or it should go into spectacle though.

STEPS TO REPRODUCE
1. Open some image in Gwenview
2. Enter annotation editor
3. Draw some stuff, write text, etc
4. Press Esc

OBSERVED RESULT

Annotation editor silently closes.

EXPECTED RESULT

If there's some input there should be a warning about unsaved data.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux
KDE Plasma Version: 5.25.5
KDE Frameworks Version: 5.97.0
Qt Version: 5.15.6
Graphics Platform: Wayland
Comment 1 Nate Graham 2022-09-13 18:18:53 UTC
It's an issue in KImageAnnotator I think; on destruction, it should ask if you want to save or discard your annotations, like most software does. You can report this at https://github.com/ksnip/kImageAnnotator/issues/
Comment 2 gudvinr+kde 2022-09-13 19:04:38 UTC
(In reply to Nate Graham from comment #1)
> It's an issue in KImageAnnotator I think; on destruction, it should ask if
> you want to save or discard your annotations, like most software does. You
> can report this at https://github.com/ksnip/kImageAnnotator/issues/

But where does it save and discard annotations?
I don't know what KImageAnnotator is, but isn't it just a component that other applications use, like text view both KWrite and Kate share?
From that perspective that sounds kinda counterintuitive and I'd rather expect either parent component handling key events and passing them down or getting notifications from component so it can take over and then show dialog. That dialog might something that application developer might want to customize, add localization, etc.

Their example doesn't look like annotation screen you see when you use e.g. Spectacle so software still does some level of control over KIA, or is it?

Sorry if that sounds intimidating, I just want to make sure that it is really upstream issue and not something that should be solved in apps use this component.
Comment 3 Nate Graham 2022-09-15 15:59:49 UTC
KImageAnnotator is a 3rd-party annotation library component.

We could override the key handling, but discarding without confirmation by default is IMO not an ideal default behavior.
Comment 4 gudvinr+kde 2022-09-15 16:28:44 UTC
(In reply to Nate Graham from comment #3)
> discarding without confirmation by
> default is IMO not an ideal default behavior.

I totally agree with that.

However, since I don't know hierarchy of things it is still not exactly clear for me how KIA is integrated into gwenview and/or spectacle.

For example, does it work with image files that were saved on disk directly or it uses some kind of intermediate interface?

In first case I'd agree that KIA should handle discard/save operation itself because it is what has "ownership" over edited images.

But if that's the latter and KIA only passes edited image over to whatever parent is and parent does all the file saving stuff, I don't think KIA is the right place to trigger any sort of dialogs.

Well, it probably should have some signals it can trigger when you close canvas but again, buttons below canvas you see in gwenview and spectacle are different so parent window does control when it closes with or without saving changes.
So, if parent window already does that, why KIA itself should care handle closing too?

I am not trying to shift responsibility, this just seems a bit counterintuitive. So I'd like to make sure that everything is sorted out and this is indeed 3rd party issue.