Note @Benoit Pierre: I am currently testing your changes (up to e6a3077758599cca7d5d675d969b135bf9024b6c "archive: improve external rar support") on top of [r1419] using Arch. For notes on [r1419] without your patches, see below. The following bugs are closely related so I present all of them at once.
unrar bug 1
Make sure that libunrar is not installed. (This ensures that mcomix uses unrar.) Open the library, go to "All books" where you must have some encrypted archives (rar archives in my case). You get password prompts. Hit cancel on each of them. Everything looks fine. Try to leave the library page (e.g. close the window, open a book from the library, switch to another collection, etc.). The GUI does not respond anymore. You might see child processes in htop, like /usr/bin/unrar vt -p -- /path/to/archive.rar. SIGKILL them (SIGTERM does not work for me) and things are (almost) fine again. Note that this happens only once per encrypted archive. (That is, if you want to test things here, you have to restart mcomix so you get asked for the password again.) Side effect: The terminal stops echoing what you are typing, it needs a reset to fix this.
unrar bug 2
Important note: Please read the whole paragraph before trying to reproduce it. Do everything described in "unrar bug 1". Now, try to drag the encrypted archive's library thumbnail. You get asked for the password again. However, this time, that thing you tried to drag is still visible and steals the focus all the time from all windows. That is, you can only interact indirectly via your window manager. If you switch to a tty, htop does not show any unrar child processes. Switch back to X and try to close the password prompt by means of your window manager (e.g. hit Alt+F4). The window might not disappear, but if you switch to tty again, you might see an unrar child process. SIGKILL it and switch back to X. Everything should work again.
libunrar bug 1
Make sure you have libunrar installed. Try the same actions as described in "unrar bug 1". Note that the GUI keeps responding but whenever you try to interact with the library in a way that "encrypted" thumbnails need to be rendered, you get asked for the passwords again and again. Note also that, if you use a window manager where the following applies, you might need to rearrange your windows to get to the password prompts at all, but rearranging windows might trigger rerendering again, so you get asked for the passwords even more often ...
libunrar bug 2
Again, make sure you have libunrar installed. Just like "unrar bug 2": Try to drag and drop an "encrypted" thumbnail. You might get asked for the password several times, depending on how often re-rendering is triggered thanks to your dragging. You can close the password prompts by simply pressing Esc, but you might need to move your mouse cursor out of the library window(!) so they do not reappear again.
It turns out that [r1419] already shows similar quirks. The most important difference is that without your patches, when using unrar, you get asked for the password only once per library opening or dragging, which is close to perfect, I think.
The library handling of password protected archive is bad, there is no way around it. Password should not be asked, and a padlock cover should be shown. All those issues are not new (except for issue 1), and some are present with 7z too.
I think the issue with 1 might be the fact that we end up calling unrar with an empty password: unrar -p archive.rar, which will result in unrar prompting for it on the terminal, hence the hang.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thank you for the patch, it indeed fixes the worst case. However, just the same way you mentioned it above, shortly after posting, I also thought that it might be best to simply disable displaying covers from password-protected archives. However, in this case, it might be helpful to replace the non-existing cover with the name of the archive in question so one can distinguish between different books. This might introduce further issues (concerning cover arrangement, for example), though. Therefore, I think we should simply leave it this (patched) way for now.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
This also contains some work to be able to set the library cover (so users can manually set the cover for encrypted archives, this would also fix [feature-requests:#65]). Right now you can drag & drop a picture from another application to a book in the library to change its cover (but not from MComix itself, like from the thumbnail bar, don't know why). You can also reset it from the library right click popup menu (and I plan to add a 'set from file' option). And there's also a small fix for a bug image_tools.load_pixbuf_size that would result in the library always trying to regenerate the cover of some of the books in my library.
Note @Benoit Pierre: I am currently testing your changes (up to
e6a3077758599cca7d5d675d969b135bf9024b6c"archive: improve external rar support") on top of [r1419] using Arch. For notes on [r1419] without your patches, see below. The following bugs are closely related so I present all of them at once.unrar bug 1
Make sure that
libunraris not installed. (This ensures that mcomix usesunrar.) Open the library, go to "All books" where you must have some encrypted archives (rar archives in my case). You get password prompts. Hit cancel on each of them. Everything looks fine. Try to leave the library page (e.g. close the window, open a book from the library, switch to another collection, etc.). The GUI does not respond anymore. You might see child processes inhtop, like/usr/bin/unrar vt -p -- /path/to/archive.rar. SIGKILL them (SIGTERM does not work for me) and things are (almost) fine again. Note that this happens only once per encrypted archive. (That is, if you want to test things here, you have to restartmcomixso you get asked for the password again.) Side effect: The terminal stops echoing what you are typing, it needs aresetto fix this.unrar bug 2
Important note: Please read the whole paragraph before trying to reproduce it. Do everything described in "unrar bug 1". Now, try to drag the encrypted archive's library thumbnail. You get asked for the password again. However, this time, that thing you tried to drag is still visible and steals the focus all the time from all windows. That is, you can only interact indirectly via your window manager. If you switch to a tty,
htopdoes not show anyunrarchild processes. Switch back to X and try to close the password prompt by means of your window manager (e.g. hitAlt+F4). The window might not disappear, but if you switch to tty again, you might see anunrarchild process. SIGKILL it and switch back to X. Everything should work again.libunrar bug 1
Make sure you have
libunrarinstalled. Try the same actions as described in "unrar bug 1". Note that the GUI keeps responding but whenever you try to interact with the library in a way that "encrypted" thumbnails need to be rendered, you get asked for the passwords again and again. Note also that, if you use a window manager where the following applies, you might need to rearrange your windows to get to the password prompts at all, but rearranging windows might trigger rerendering again, so you get asked for the passwords even more often ...libunrar bug 2
Again, make sure you have
libunrarinstalled. Just like "unrar bug 2": Try to drag and drop an "encrypted" thumbnail. You might get asked for the password several times, depending on how often re-rendering is triggered thanks to your dragging. You can close the password prompts by simply pressingEsc, but you might need to move your mouse cursor out of the library window(!) so they do not reappear again.It turns out that [r1419] already shows similar quirks. The most important difference is that without your patches, when using
unrar, you get asked for the password only once per library opening or dragging, which is close to perfect, I think.Related
Commit: [r1419]
The library handling of password protected archive is bad, there is no way around it. Password should not be asked, and a padlock cover should be shown. All those issues are not new (except for issue 1), and some are present with 7z too.
I think the issue with 1 might be the fact that we end up calling unrar with an empty password:
unrar -p archive.rar, which will result in unrar prompting for it on the terminal, hence the hang.I pushed a fix for issue 1.
Thank you for the patch, it indeed fixes the worst case. However, just the same way you mentioned it above, shortly after posting, I also thought that it might be best to simply disable displaying covers from password-protected archives. However, in this case, it might be helpful to replace the non-existing cover with the name of the archive in question so one can distinguish between different books. This might introduce further issues (concerning cover arrangement, for example), though. Therefore, I think we should simply leave it this (patched) way for now.
Yes, agreed. I'm working on a branch to do that: https://github.com/benoit-pierre/mcomix/compare/improve-library
This also contains some work to be able to set the library cover (so users can manually set the cover for encrypted archives, this would also fix [feature-requests:#65]). Right now you can drag & drop a picture from another application to a book in the library to change its cover (but not from MComix itself, like from the thumbnail bar, don't know why). You can also reset it from the library right click popup menu (and I plan to add a 'set from file' option). And there's also a small fix for a bug
image_tools.load_pixbuf_sizethat would result in the library always trying to regenerate the cover of some of the books in my library.Related
Feature Requests: #65