Add "Open Containing Folder" feature
ClosedPublic

Authored by ngraham on Jun 28 2019, 3:44 PM.

Details

Summary

This patch adds an "Open Containing Folder" menu item to the file menu, similar to
other KDE apps like Gwenview that have it there. This action is especially useful
for the case when you've downloaded a PDF from the internet that opens itself in
Okular. The location of this file may not be clear or easy to find without this
feature, and on several occasiona I have found myself wishing for it when this
happens.

Test Plan
  • Delete or move aside ~/.local/config/kxmlgui5/okular/part.rc
  • Menu item is enabled and works when there is an open document:
  • Menu item is disabled when there is no open document:

Diff Detail

Repository
R223 Okular
Branch
add-open-containing-folder-feature (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14045
Build 14063: arc lint + arc unit
ngraham created this revision.Jun 28 2019, 3:44 PM
Restricted Application added a project: Okular. · View Herald TranscriptJun 28 2019, 3:44 PM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
ngraham requested review of this revision.Jun 28 2019, 3:44 PM

How does it work when the document is not local, or is deleted?
E. g. do

okular https://20years.kde.org/book/20yearsofKDE.pdf
part.cpp
3712

Did someone test this with an encrypted PDF file?

ngraham planned changes to this revision.Jul 5 2019, 1:14 PM

Good points!

ngraham updated this revision to Diff 61216.Jul 5 2019, 2:04 PM

Show the local file path, which handles the case of locally-downloaded remote files

I don't have any encrypted PDFs to test. If you have any, could you send one along? Or better yet, test it with this feature?

Show the local file path, which handles the case of locally-downloaded remote files

That makes sense to me.

I don't have any encrypted PDFs to test. If you have any, could you send one along? Or better yet, test it with this feature?

Don’t have any, but maybe the one who implemented encrypted file handling?
I don’t have a wallplug nearby again, so I will not do much stuff that requires compiling the next time.

aacid added inline comments.Jul 6 2019, 10:57 AM
part.cpp
3712

Encrypted files are just files, why would they be any different?

Anyhow https://gitlab.freedesktop.org/poppler/test/raw/master/unittestcases/PasswordEncrypted.pdf?inline=false + "password" if you feel like testing it

Works fine with that encrypted PDF.

I believe this is ready for review and merging.

And since this contains a new string, if possible I'd like to try to get it into the next release before the freeze, which is in three days.

Shouldn't this be below Open Recent?

Actually now that I think about it, it should be even lower in the menu since it isn't an action that results in the opening of a new document, it's an action that gets performed on an open document.

ngraham updated this revision to Diff 61826.Jul 15 2019, 7:33 PM

Put it in the group of other actions that act on the open file

ngraham marked 2 inline comments as done.Jul 15 2019, 7:34 PM

Sorry, I forgot about this.

realUrl() does not behave special with encrypted files, but with compressed files. That was my mistake.

When the file is not compressed, realUrl() is not updated. This can be tested by opening a compressed file (do gzip on a PDF file) and then replacing the compressed file by an uncompressed file. The file will be reloaded, but the window title text does not get updated, because it uses realUrl().

Since you are not using realUrl() anymore, this is not important here. :)

Is that a shipit? :)

ngraham updated this revision to Diff 61914.Jul 17 2019, 1:58 PM

Don't crash if setupActions() didn't get run

aacid accepted this revision.Jul 17 2019, 2:17 PM
This revision is now accepted and ready to land.Jul 17 2019, 2:17 PM
This revision was automatically updated to reflect the committed changes.