Bug 425354 - In recent version 1.11 the actions are mostly wrong
Summary: In recent version 1.11 the actions are mostly wrong
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 1.11.0
Platform: Flatpak Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords: usability
Depends on:
Blocks:
 
Reported: 2020-08-14 17:57 UTC by Gabriel
Modified: 2020-08-30 17:28 UTC (History)
4 users (show)

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


Attachments
.config/okularpartrc breaks okular annotations (1.90 KB, text/plain)
2020-08-21 04:13 UTC, Drew Parsons
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gabriel 2020-08-14 17:57:10 UTC
SUMMARY

(I'm translating from Spanish, so some of the names may differ slightly)

1. The "Quick annotations" no longer stays fixed to the side of the document when clicked. Now it's a pop-up menu that hides after selecting a tool. Is this by design? Because it's very inconvenient.
2. "Underline" is a text note
3. "Strike through" is "Underline"
4. "Typewriter" is an emergent note
5. "Emergent note" is the action to draw free-hand
6. Arrows do nothing
....


SOFTWARE/OS VERSIONS
Linux/KDE Plasma: elementary OS 5.1.7 Hera (based on Ubuntu 18.04.4)
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 5.70.0
Qt Version: 5.14.2

ADDITIONAL INFORMATION
Comment 1 Laura David Hurka 2020-08-15 11:43:18 UTC
> 1. The "Quick annotations" no longer stays fixed to the side of the document when clicked. Now it's a pop-up menu that hides after selecting a tool. Is this by design? Because it's very inconvenient.

That is intended. It should be possible to float the whole new annotation toolbar. Right-click on the toolbar -> Unlock Toolbars, then you should be able to dock it to the right edge of the screen to make it vertical, then move it where you need it.

Unfortunately on my system it becomes horizontal when I undock it, and it appears on every screen then. So that is not optimal...
Comment 2 Gabriel 2020-08-15 13:03:31 UTC
> It should be possible to float the whole new annotation toolbar. Right-click on the toolbar -> Unlock Toolbars, then you should be able to dock it to the right edge of the screen to make it vertical, then move it where you need it.

I do this, but in the previous version the "Quick annotations" buttons remained fixed to the left side of the screen after a single click. Now I have to click every time I want to edit the PDF. This is extremely inconvenient.
Comment 3 Drew Parsons 2020-08-21 04:13:37 UTC
Created attachment 131066 [details]
.config/okularpartrc breaks okular annotations

I had much the same report on a Debian system, reported at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=968438

The label on the horizontal buttons was not matching the
function triggered by the button. A different popup hint appeared
*after* you click on the button, with different text that matches the
actual function.  e.g. Click on "Highlighter", a Popup Note is
triggered. Click on "Popup Note", "Put a stamp symbol" is triggered etc.

The Debian maintainers suggested it might be a problem with config files, and so it proved to be.  You use two config files, .config/okularrc but also 
.config/okularpartrc

I find I can reproduce (and eliminate) the bug by removing the second 
config file .config/okularpartrc. Removing it, the functions operate as labelled. Reinstating it, the functions break as reported.

So removing  .config/okularpartrc  works around the bug.

I'm attaching the okularpartrc which breaks the annotation functions in the new version of okular
Comment 4 Laura David Hurka 2020-08-21 12:10:43 UTC
> I'm attaching the okularpartrc which breaks the annotation functions
> in the new version of okular

Thanks. It is missing the key QuickAnnotationTools, which means that the update script didn’t do its job. Maybe it doesn’t run on Elementary OS or in Flatpak?

Or Flatpak ships the Release Candidate (RC) instead of the Release? The update script in RC didn’t work correctly yet.
Comment 5 Albert Astals Cid 2020-08-22 22:54:53 UTC
(In reply to David Hurka from comment #4)
> > I'm attaching the okularpartrc which breaks the annotation functions
> > in the new version of okular
> 
> Thanks. It is missing the key QuickAnnotationTools, which means that the
> update script didn’t do its job. Maybe it doesn’t run on Elementary OS or in
> Flatpak?

It is really not acceptable to be broken if the update script doesn't run. The only acceptable outcome is "some configuration got lost" but "things are not working anymore" is terrible, why did plural you think that was acceptable?

> 
> Or Flatpak ships the Release Candidate (RC) instead of the Release? The
> update script in RC didn’t work correctly yet.

Why would flatpak ship the wrong code? That simply makes no sense.
Comment 6 Albert Astals Cid 2020-08-22 22:57:13 UTC
Not it's true that flatpak is probably not running the update code because since everything is confined in its own hidden, place the .upd file is not seen by the update system, but as said, the code should not break if the config files are not "as expected", that's just wrong coding.
Comment 7 Laura David Hurka 2020-08-23 13:37:08 UTC
(In reply to Albert Astals Cid from comment #6)
> the code should not break if the
> config files are not "as expected", that's just wrong coding.

Our use of kconf_update is to make the config files “as expected”, but if that doen’t work reliable, it is understandable to require the code to be tolerant about kconf_update not working.

I think the simplest solution is to not use the key AnnotationTools but e. g. LiveAnnotationTools. This key would be new, so there can’t be old tools in its place. The user would lose the live configuration, but that is probably a minor issue, since it is “live”.
Comment 8 Simone Gaiarin 2020-08-24 07:58:55 UTC
I agree on the solution proposed by David of renaming the key 'AnnotationTools' to something else. I should have done this in the first place given that it has no drawbacks, and avoids the issue reported here. I will submit a MR soon with this fix.

Just a note, besides flatpack, the other main reason why people experienced this issue is because they tested a pre-RC version of okular. This caused the update script to run while it was still buggy. The problem is that once it was run once, it will not run again, so that when the stable version of okular was released, for those users the correct update script is not run and they are left with the wrong configuration. This just to say that I expect that for most of the users running only stable versions of okular there should be no problem while we fix this.
Comment 9 Bug Janitor Service 2020-08-25 08:27:51 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/252
Comment 10 Simone Gaiarin 2020-08-30 17:28:56 UTC
Git commit 10d92fbeda39d3b177b0681e68ff1f5ea6b99270 by Simone Gaiarin.
Committed on 25/08/2020 at 08:16.
Pushed by aacid into branch 'release/20.08'.

Store the state of the builtin annotations in a new config key

Using the new configuration key BuiltinAnnotationTools instead of AnnotationTools, we avoid any conflicts in the configuration files due to the fact that the key AnnotationTools had a different meaning in the previous versions of Okular. In particular we avoid the critical problem that the actions in the UI do not match the actual annotation tools. The conflict may happen if the kconf_update script is not executed for some reason (e.g. okular running from flatpack).
FIXED-IN: 1.11.1

M  +5    -5    conf/okular.kcfg
M  +6    -0    okular.upd
M  +13   -13   ui/pageviewannotator.cpp
M  +4    -4    ui/pageviewannotator.h

https://invent.kde.org/graphics/okular/commit/10d92fbeda39d3b177b0681e68ff1f5ea6b99270