Bug 424810 - Assert crash when selecting quick annotation tool that was added in the config dialog
Summary: Assert crash when selecting quick annotation tool that was added in the confi...
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 1.10.80
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-29 23:13 UTC by Laura David Hurka
Modified: 2020-08-05 22:50 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.11.0
Sentry Crash Report:


Attachments
Assert crash backtrace after adding Stamp tool. (11.06 KB, text/plain)
2020-07-29 23:13 UTC, Laura David Hurka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Laura David Hurka 2020-07-29 23:13:45 UTC
Created attachment 130497 [details]
Assert crash backtrace after adding Stamp tool.

SUMMARY
I added the default Stamp tool (despite warning) to Quick Annotations, and selected it.

STEPS TO REPRODUCE
1. Add tool: Quick Annotations -> Configure...
2. Use tool: Quick Annotations -> Stamp [Alt+7]

OBSERVED RESULT
Crash: See backtrace

EXPECTED RESULT
Makes useless Stamp annotations, so I can test transparency

SOFTWARE/OS VERSIONS
KDE Neon 5.19
KDE Plasma Version: 5.19.2
KDE Frameworks Version: 5.72
Qt Version: 5.14.2

First idea is that AnnotationTools is not updated at configChanged(), but only at “Add to Quick Annotations”. But the crash persists after restart, and produces the same backtrace. It still seems like tool() returns a null QDomNode, since QList<QAction*>::at(-2) is called.

When I add another Stamp tool from “Add to Quick Annotations”, only the second Stamp tool works.

Apparently the same is true for other tool types.
Comment 1 Albert Astals Cid 2020-07-30 22:15:02 UTC
Same here.
Comment 2 Simone Gaiarin 2020-07-31 09:10:12 UTC
Git commit 280fe3c7a211a1e19f1bd4b602b7402a471f9931 by Simone Gaiarin.
Committed on 31/07/2020 at 07:50.
Pushed by gaiarin into branch 'fix-424810'.

Define sourceId when creating quick tools from Okular settings

Only the quick annotation tools created using the "bookmark" action were correctly assigned the sourceId. Now also those created in the Okular settings page have this attribute correctly defined.

Without this field it was not possible to identify which built-in annotation tool action to check when the corresponding quick tool action is triggered by the user.
FIXED-IN: 1.11.0

M  +18   -0    conf/editannottooldialog.cpp

https://invent.kde.org/graphics/okular/commit/280fe3c7a211a1e19f1bd4b602b7402a471f9931
Comment 3 Laura David Hurka 2020-07-31 09:30:08 UTC
> Pushed by gaiarin into branch 'fix-424810'.
:)

> +        if (la->lineStartStyle() != 5 || la->lineEndStyle() != 5) {
Since lineStartStyle() returns the enum directly, use “None” instead of “5”?
Comment 4 Simone Gaiarin 2020-07-31 09:43:48 UTC
I wrongly pushed my branch to upstream instead that on my fork and I think it triggered something here.

I have removed the branch from upstream and created it on my fork now.

Hopefully I have not messed up anything.
Comment 5 Albert Astals Cid 2020-07-31 23:05:11 UTC
(In reply to Simone Gaiarin from comment #4)
> I wrongly pushed my branch to upstream instead that on my fork and I think
> it triggered something here.
> 
> I have removed the branch from upstream and created it on my fork now.
> 
> Hopefully I have not messed up anything.

If you push upstream, push under work/ i.e. work/gaiarin/fix_blabla and then triggers are ignored.
Comment 6 Albert Astals Cid 2020-08-05 22:50:01 UTC
Git commit 72f5a2db089178c1e9eca82ba5a9a97fff2a49dc by Albert Astals Cid, on behalf of Simone Gaiarin.
Committed on 05/08/2020 at 22:49.
Pushed by aacid into branch 'release/20.08'.

Find built-in tool corresponding to quick tool at runtime

In this way it is possible to drop the `sourceId` attribute from the quick tools definition. This simplifies the code logic and makes it easier to update user settings from the previous version of Okular (because there is no need to add the attribute `sourceId`

This also fixes the crash due to the fact that `sourceId` was not correctly created when a quick annotation is created from the Annotation page of Okualr Settings.
FIXED-IN: 1.11.0

M  +1    -1    autotests/CMakeLists.txt
M  +19   -2    autotests/annotationtoolbartest.cpp
M  +30   -5    ui/annotationactionhandler.cpp
M  +5    -0    ui/data/tools.xml
M  +12   -6    ui/data/toolsQuick.xml
M  +17   -3    ui/pageviewannotator.cpp

https://invent.kde.org/graphics/okular/commit/72f5a2db089178c1e9eca82ba5a9a97fff2a49dc