Implement the generation of a custom Info.plist for the Mac OS bundle
AbandonedPublic

Authored by sbragin on Feb 24 2018, 7:59 PM.

Details

Reviewers
rjvbb
aacid
Group Reviewers
Okular
Summary

This patch implements all Info.plist properties, recommended by Apple
(except NSMainNibFile), and adds corresponding entries for handling
PDF, CHM, DjVu, DVI, EPUB, FictionBook, image, ODT, plain text, PS,
and XPS files.

Original author: René J.V. Bertin <rjvbertin@gmail.com>

Diff Detail

Repository
R223 Okular
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
sbragin created this revision.Feb 24 2018, 7:59 PM
Restricted Application added a project: Okular. · View Herald TranscriptFeb 24 2018, 7:59 PM
Restricted Application added a subscriber: Okular. · View Herald Transcript
sbragin requested review of this revision.Feb 24 2018, 7:59 PM
sbragin added a subscriber: rjvbb.

Comment #1: Icons for file types, associated with Okular, are missing. It has been left for a future patch.
Comment #2: Not all supported formats have been added to the Info.plist file.

rjvbb accepted this revision.Feb 25 2018, 11:23 AM

I can hardly NOT accept this, given I'm the original author :)

Adding document-type icons isn't strictly necessary. The Finder will display documents with the icon of the default application configured to open the type in question, or with the application configured for the individual document. Applications that provide icons for the different document types they support usually do this by adapting the application icon instead of using a generic icon for the document type (exceptions aside of course). (I'm not at my Mac right now and hardly use the Finder in practice; I wouldn't be surprised if the Finder actually tagged application icon with a standard document type representation when apps don't provide their own icons.)
That means it wouldn't be appropriate to bundle the document-type icons from a standard Freedesktop.org theme (let alone from Breeze, IMHO)

This revision is now accepted and ready to land.Feb 25 2018, 11:23 AM
aacid requested changes to this revision.Feb 25 2018, 11:29 AM
aacid added a subscriber: aacid.

This is wrong because lots of the support for those file formats depends whether you have the backends or not, so this either should be made to depend on which dependencies where found when running cmake or each backend plugin should install its own file (if macos supports that).

This revision now requires changes to proceed.Feb 25 2018, 11:29 AM

This is wrong because lots of the support for those file formats depends whether you have the backends or not, so this either should be made to depend on which dependencies where found when running cmake or each backend plugin should install its own file (if macos supports that).

Good point. I will think how to fix it.

I think that this was already discussed, but would it make sense to have a function in extra-cmake-modules which generates the plist file based on some parameters (known from somewhere else or passed explicitly) instead of hand-writing that file?

Adding document-type icons isn't strictly necessary. The Finder will display documents with the icon of the default application configured to open the type in question, or with the application configured for the individual document. Applications that provide icons for the different document types they support usually do this by adapting the application icon instead of using a generic icon for the document type (exceptions aside of course). (I'm not at my Mac right now and hardly use the Finder in practice; I wouldn't be surprised if the Finder actually tagged application icon with a standard document type representation when apps don't provide their own icons.)

If I associate, e.g., PDF's with Okular, then Finder shows small previews instead of icons, which is probably the default behavior.

That means it wouldn't be appropriate to bundle the document-type icons from a standard Freedesktop.org theme (let alone from Breeze, IMHO)

Indeed.

I think that this was already discussed, but would it make sense to have a function in extra-cmake-modules which generates the plist file based on some parameters (known from somewhere else or passed explicitly) instead of hand-writing that file?

For me it makes sense for properties that are more or less frequently changing, like the version, or the year of the copyright. And CMake itself supports this. Other properties can be hardcoded, in my opinion, unless you are going to start changing the name of the application once per month or something.

I think that this was already discussed, but would it make sense to have a function in extra-cmake-modules which generates the plist file based on some parameters (known from somewhere else or passed explicitly) instead of hand-writing that file?

But this would make perfect sense, if we want to insert the list of supported file formats dynamically, depending on libraries found in the system.

rjvbb added a comment.Feb 25 2018, 1:05 PM

My original implementation is used in conjunction with a build system that ensures all dependencies are available.

I don't know if cmake is flexible enough to add the XML elements for the document types that a local build will support given the locally available dependencies, without having to generate the entire file from within a CMake file. The functionality present in the ECM uses a template file containing tokens (variables) that are replaced with the appropriate strings during the configure phase. The task at hand seems more complex and I have my doubts as to how worth it would be the effort to implement support for doing this in the ECM. I'd say there are Mac-specific things that should be higher on the list.

In case of Okular one could think of printing a big CMake warning (on Mac), or even let CMake fail by default if all dependencies implied by the plist file are not available.

Note that this just tells LaunchServices what an application can or could do. It does not in any way modify anything else, in particular it won't change the application the system will use to open a document type it already knows how to open. Also note that we're not talking about a platform where distribution systems packages broken down in fine-grained bits and pieces and where you can easily end up with a partial Okular install.

If Okular's capabilities depend on the availability of plugins which might be installed independently then the current implementation is acceptable and comparable to Apple's own policies with e.g. QuickTime Player (which at least in the past would often tell the user that it needed additional components to open a given video file). Annoying for common file types, but competely moot for filetypes you won't ever encounter.

Finally, Kate and Kdevelop follow the same "hard-wired" principle, the former using a wildcard solution ("this app can edit anything"). Not optimal maybe, but not a true issue for applications that the user either installs from an official all-inclusive app-bundle download, or using MacPorts or HomeBrew packaging, or else from a manual build-from-scratch.

I think that this was already discussed, but would it make sense to have a function in extra-cmake-modules which generates the plist file based on some parameters (known from somewhere else or passed explicitly) instead of hand-writing that file?

For me it makes sense for properties that are more or less frequently changing, like the version, or the year of the copyright. And CMake itself supports this. Other properties can be hardcoded, in my opinion, unless you are going to start changing the name of the application once per month or something.

Not only that: think about a macro like ecm_add_app_icon:
https://api.kde.org/ecm/module/ECMAddAppIcon.html

It is a noop on platforms different from Windows and macOS, but it was added to most of our programs, even by programmers who don't usually works on those platforms. But people working on them have the feature enabled "for free". So an hypothetical macro ecm_add_plist_do_some_magic would probably gain some traction also by people not involved with macOS, who don't know who to create a plist file, IMHO.

rjvbb added a comment.Feb 25 2018, 1:58 PM

Then again, all "not non-gui" applications need an application icon, and adding one is simple enough that it can be done in cmake with a few simple enough operations. And a little bit of extra care in the source code to avoid undoing the effect of ecm_add_app_icon.

Adding supported document types in the plist has zero interest if you don't modify the code so it catches Qt's FileOpen events or even implement an independent handling of the underlying platform events (Qt's generic interface is limited). That's not any easier than writing a plist (for which Xcode provides an editor that takes care of the XML generation).

Let's approach this from the other direction. ecm_add_app_icon and (IIRC) ecm_mark_nongui_application both use existing cmake functionality to generate the desired plist file from a template. The template plist contains tokens as the value of a number of important keys. It still becomes a valid plist if you don't replace all of those template tokens (e.g. because a project's build system doesn't provide all the required information). What support does cmake have to add arbitrary XML sections to a template file?

Suppose you'd need to create a block (list of sections to insert) and then use that as the replacement for a token that's at the appropriate location in the template plist, which would be the easy approach. What happens that token isn't replaced? The plist wouldn't be formally valid XML, but how would the OS treat such files?

I'm not saying it should never be done. I do have my doubts as to the urgency and the return on investment.

sbragin added a comment.EditedFeb 25 2018, 2:19 PM

The task at hand seems more complex and I have my doubts as to how worth it would be the effort to implement support for doing this in the ECM. I'd say there are Mac-specific things that should be higher on the list.

I agree, especially because a bundle must contain all dependencies by definition, and the problem in question is odd from this perspective. But since no one is providing official bundles of Okular, the only way is compiling from the source, and then everything depends on which configuration a user have. It is true, that if one uses your MacPorts repository, then everything is consistent. Probably, it would be reasonable to push Info.plist without the document type implementation, and postpone the latter indefinitely. Then everything would work almost normally, apart from some minor inconveniences:

  • Okular would not be shown in the list "Open with";
  • a user would need to set Okular as the default app for each desired file format manually, and also go through the warning, that the application may not support a chosen file type.

Adding supported document types in the plist has zero interest if you don't modify the code so it catches Qt's FileOpen events

I decided to divide it into two parts, FileOpen event handling comes with the next patch. It still makes sense to change Info.plist (without this, e.g., no file type can be associated with Okular at all).

sbragin abandoned this revision.Dec 1 2018, 2:35 PM
Restricted Application added a subscriber: okular-devel. · View Herald TranscriptDec 1 2018, 2:35 PM