Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REGRESSION] Opening app with ZIM doesnt open ZIM #962

Open
rgaudin opened this issue Sep 2, 2024 · 15 comments · Fixed by #968
Open

[REGRESSION] Opening app with ZIM doesnt open ZIM #962

rgaudin opened this issue Sep 2, 2024 · 15 comments · Fixed by #968
Assignees
Labels
Milestone

Comments

@rgaudin
Copy link
Member

rgaudin commented Sep 2, 2024

When opening Kiwix by double-cliking a ZIM file or using open command, the App starts and displays the last viewed ZIM.

I am expecting the clicked ZIM to be opened instead

@rgaudin rgaudin added the bug label Sep 2, 2024
@rgaudin
Copy link
Member Author

rgaudin commented Sep 2, 2024

macOS 14.6.1 / Kiwix 3.5.1

@kelson42
Copy link
Contributor

kelson42 commented Sep 2, 2024

@rgaudin @BPerlakiH Do we know if this is a regression?

@kelson42 kelson42 added this to the 3.6.0 milestone Sep 2, 2024
@rgaudin
Copy link
Member Author

rgaudin commented Sep 2, 2024

Well we had #699 and #850

@kelson42
Copy link
Contributor

kelson42 commented Sep 2, 2024

OK, so this is a regression. To be fixed in priority!

@kelson42 kelson42 changed the title Opening app with ZIM doesnt open ZIM [REGRESSION] Opening app with ZIM doesnt open ZIM Sep 2, 2024
@kelson42 kelson42 modified the milestones: 3.6.0, 3.5.2 Sep 3, 2024
@BPerlakiH
Copy link
Collaborator

I managed to reliably reproduce it.
There are 2 ways to exit the macOS application:

  1. closing all windows (one by one), either via the red X system button (top left corner of the window) or pressing CMD+W on each window. Once the last window is closed, the app will exit.
  2. Regardless of the amount of windows open, pressing CMD+Q, will exit the app immediatelly.

So what is the difference?
In case 1) we close the window, and delete the tab / window data, as the user closed it, so it's no longer needed.
In case 2) we save data from the windows, in order to restore it on next launch. This mechanism of restoring the windows is not fully working.
There's a built in SwiftUI way to handle this specific feature, but so far it was working only for so called "Document based app".

@kelson42
Copy link
Contributor

kelson42 commented Sep 7, 2024

@BPerlakiH We should secure that both ways of quitting trigger the same quit() procedure where we do our stuff.

@BPerlakiH
Copy link
Collaborator

BPerlakiH commented Sep 7, 2024

So the way I see it is:
We can have multiple windows, therefore closing a window, and deleting (not storing) any data related to that, is the correct behaviour.

  • closing the last window, closes the application, this is also correct.
  • saving the state of the windows and restoring it could be a nice to have feature, but it's not working properly on macOS. I checked it wasn't fully working (not even in the oldest available TestFlight build: 3.4.0 (163)). This implementation is restoring only 1 window, and not all of them, which leads to this bug.

For these reasons, I am proposing, to remove this semi-working window restore solution we have, and by that we fix this specific bug.

Then create a new issue, to implement the windows management for macOS properly. It's really an edge case, since the only way to use this feature is to use CMD+Q or from the App Menu choose Quit Kiwix.

Screenshot 2024-09-07 at 11 50 29

IMHO and from my "user perspective", the application not "remembering" my "windows" is not a deal breaker, more important is to be able to quickly find content (such as "global search"), or as in this case getting to the content of the ZIM file from Finder.

@rgaudin
Copy link
Member Author

rgaudin commented Sep 19, 2024

Don't know how that's possible as the fixing PR was tested by me and worked but using released 3.5.2, it's not working.

@rgaudin rgaudin reopened this Sep 19, 2024
@kelson42
Copy link
Contributor

@BPerlakiH Top Prio. Would be really great if you find a way to ensure this is the last time we reopen the issue?
image

@BPerlakiH
Copy link
Collaborator

Don't know how that's possible as the fixing PR was tested by me and worked but using released 3.5.2, it's not working.

I have double checked it, and downloaded the App Store version, and it works for opening ZIM files, both when the app is closed, and when the app is already opened.

Maybe worth to double check if you don't have another copy of an older application, or clear the folder before installing.

@rgaudin
Copy link
Member Author

rgaudin commented Sep 20, 2024

I checked before reopening and I don't. I am using the DMG distributed version though.

On a second thought, I find it unreasonable to prevent users from having multiple versions on the app anyway. Double-clicking a ZIM ends up launching on app, so that app has been found by the system and was probably passed the filename.
If there's a macOS bug regarding this, we should gather evidence online or open a ticket upstream

@BPerlakiH
Copy link
Collaborator

Ok, we narrowed it down during our call with @rgaudin , it seems to be a macOS Sequoia (15.0) issue.

@BPerlakiH
Copy link
Collaborator

I will be updating to Sequoia, just wanted to wait a bit with it, as there's no way back (downgrade), so won't be able to test the apps on "older" macOS.

@kelson42 kelson42 modified the milestones: 3.5.2, 3.6.0, 3.7.0 Nov 4, 2024
@kelson42
Copy link
Contributor

kelson42 commented Nov 4, 2024

@BPerlakiH How does it look like now?

@benoit74
Copy link

benoit74 commented Nov 5, 2024

I will be updating to Sequoia, just wanted to wait a bit with it, as there's no way back (downgrade), so won't be able to test the apps on "older" macOS.

I'm still on Sonoma, and I will stay on it until this issue is fixed, so that we can still test it on old MacOS. Unless this takes months, but I hope it will be fixed soon ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants