Opened files affect system menu entries

[Vit]

I stumbled across a very interesting bug, it seems that opened file tabs actually affect system menu entries and may cause them to malfuction when the name matches.

1. Open setup.pdf in qpdfview (attached).
2. Open any other file and switch to its tab.
3. Click qpdfview → Preferences to open settings window.
4. Close settings window.
5. Click qpdfview → Preferences to open settings window again.
6. Close setup.pdf tab.
7. Click qpdfview → Preferences to open settings window once more.

What actually happens is that:
- At step 5 instead of the settings window qpdfview will jump to the opened setup.pdf tab.
- At step 7 there will be no Preferences entry in the menu, since closing setup.pdf removes it.

This issue seems to be macOS-specific and might actually be Qt-specific as I cannot reproduce it on Ubuntu.

[Vit (vit9696)]

I believe I was able to identify the relevant code that breaks it[1]. All the filenames in this piece of code (Preferences, Setup, etc.) cause this insane behaviour. Interestingly, it is not reproducible with About, Cut, or any others. Perhaps the code path in the other place matters somehow.

I created a hack[2] that seems to work for me, but I am not sure whether you would want to include it upstream.

[1] https://github.com/qt/qtbase/blob/1869615f/src/plugins/platforms/cocoa/qcocoamenuitem.mm#L228-L234
[2] https://github.com/vit9696/qpdfview/blob/master/patches/tabname.diff

[Adam Reichold (adamreichold)]

Hello Vit,

thank you for taking the time to report this and for figuring out a workaround! I would be willing to carry the workaround upstream, I do however wonder whether it can be simplified? For example, couldn't we just always prepend a zero-width space on macOS without checking the tab title based on the translations? This should hopefully achieve the same effect with much less moving parts.

Best regards,
Adam

[Vit (vit9696)]

Hi Adam,

Yes, I can confirm that it works as well and, to be honest, this might be even safer, given the mess Qt devs left us with. I updated the patch[1] and can confirm it still works fine.

Actually, I am slowly nailing down some other bugs I noticed earlier over the time of using qpdfview on macOS, which I believe would also be great to include upstream[2][3]. Still, fullscreen and presentation view on macOS are really messed up, and not just in qpdfview, thanks to Qt. Additionally I set up the CI with automatic binary uploading to make it more transparent and easy to test.

Best,
Vit

[1] https://github.com/vit9696/qpdfview/blob/master/patches/tabname.diff
[2] https://github.com/vit9696/qpdfview/blob/master/patches/fix-dock-titles.diff
[3] https://github.com/vit9696/qpdfview/blob/master/patches/fontconfig.diff

[Adam Reichold (adamreichold)]

Hello again,

I just pushed trunk revision 2114 which contains the tab name and dock title fixes. I am not to keen on the environment variable changes for fontconfig in their current form. Can't this be injected into the process environment without code changes?

Best regards,
Adam

[Adam Reichold (adamreichold)]

Make that trunk revision 2115 as I mistyped and broke the build which I did not noticed because I compiled with the preprocessor directives commented out...

[Vit (vit9696)]

Thanks for the changes. Looks good.

As for fontconfig, this is due to how the code is packaged, as on macOS you usually package applications in self-contained bundles (apps), which fontconfig does not work with that natively. Apparently in the past there was a dedicated Info.plist entry to set the variables, but it is both broken and does not support relative paths. Using any executables (e.g. shell) prior to running the main one also causes various issues.

For this reason it seems that using setenv is the only way to do it, and it is rather common in fact[1]. The special part to it is that it depends on the way qpdfview is packaged actually. Perhaps, for this very reason, it may make sense to keep the patch local to my repo with the scripts.

[1] https://github.com/search?l=Objective-C&p=1&q=FONTCONFIG_PATH&type=Code