search

kiwix / libkiwix

Common code base for all Kiwix ports
https://download.kiwix.org/release/libkiwix/
GNU General Public License v3.0
112 stars 54 forks source link

To import bookmarks in android we require the `library.xml` #1069

Closed MohitMaliFtechiz closed 1 month ago

MohitMaliFtechiz commented 3 months ago

Describe the bug

We have implemented a feature in our "Kiwix Android" application https://github.com/kiwix/kiwix-android/pull/3724 to export the saved bookmarks so that user can import these bookmarks later (After reinstalling the application). But here we need the library.xml file as well along with the bookmarks.xml because library.xml contains the book-related data(favicon and ZIM file path). We need these two things after importing the bookmarks since without these things we are not able to reopen the bookmarks.

Describe the solution would you like

If we can save the favicon and ZIM file path in Bookmark that would be great. It will save the user's device memory(then we does not need to export the library.xml file) and make it easy to import the bookmarks as he only needs to select the bookmark.xml, and all the bookmarks will be imported into the application.

kelson42 commented 1 month ago

If a bookmark can not be attached to a book in the library, then this a kind of invalid bookmark (at least athe moment)... so I don't see what is the problem of not having a logo or ild ZIM path?!

To attach a bookmark to a library ZIM, the ZIM metadat id should be used.

rgaudin commented 1 month ago

@kelson42 what about following scenario:

Unless it's written when removing a ZIM file from my local library that it will delete all associated data (mentioning bookmarks!) or that exporting bookmarks only exports bookmarks for currently in library I think users will expect all their bookmarks to be exported.

kelson42 commented 1 month ago

@rgaudin Agree, but don't see how your comment is related to this issue.

To me, it makes little sense to display invalid bookmarks. So you don't need a logo. But tgat does not mean they should be removed.

rgaudin commented 1 month ago

@rgaudin Agree, but don't see how your comment is related to this issue.

This in in response to “If a bookmark can not be attached to a book in the library, then this a kind of invalid bookmark” which kind of implies it will not be exported.

To me, it makes little sense to display invalid bookmarks. So you don't need a logo. But tgat does not mean they should be removed.

My point. Invalid ones are not to be displayed but should be kept in case they become valid again.

kelson42 commented 1 month ago

@MohitMaliFtechiz Any feedback on this?

kelson42 commented 1 month ago

@MohitMaliFtechiz We need a feedback here, otherwise I will have to close the ticket.

MohitMaliFtechiz commented 1 month ago

@rgaudin I had opened this ticket to ease the importing process of the bookmarks for the user. Please see these comments https://github.com/kiwix/kiwix-android/issues/3724#issuecomment-1987904976, https://github.com/kiwix/kiwix-android/issues/3724#issuecomment-1988040082 since there are two files library.xml, and bookmarks.xml for properly importing the bookmarks. We are exporting the library.xml only for retrieving the zimFilePath and favicon because we need this zimFilePath for opening the ZIM file when a user tries to click on the saved bookmark(After importing the bookmarks).

My point. Invalid ones are not to be displayed but should be kept in case they become valid again.

In importing the bookmarks, currently, the user has to import both files, otherwise, all the bookmarks become invalid because the zimFilePath and favicon are in the library.xml. Importing both files is not a big deal(we can handle it on the Android side). But look at the other side of this, I have saved 10 bookmarks in my application, and the bookmarks.xml size is 4KB, but the library.xml has a 64KB size, and keeping this in storage only for favicon and zimFilePath is bit expensive for storage(since this file can be in MBs when user save more bookmarks in different ZIM files).

Screenshot from 2024-06-03 12-07-09 Screenshot from 2024-06-03 12-06-43

My point. Invalid ones are not to be displayed but should be kept in case they become valid again.

@rgaudin Let's think about the user's point of view. A user exported all the bookmarks, and he uninstalled the application and reinstalled it after some time. In the beginning, he imports the bookmarks, and goes into the bookmarks section, at this point all the bookmarks are invalid he can not see any bookmarks there but they are kept in the background. So the user will be confused about whether the bookmarks are imported or not(Might be he will try again to import the bookmarks but the result will be the same). I also respect your perspective, but at least the favicon should be saved in the bookmark so that we can show it to the user(or it would be nice if we save both favicon and zimFilePath to save the user device's storage).

The bookmark will be valid when the same book is added to the library, and this will happen when we open the ZIM file in the reader.

I was thinking about this, If there is any good reason behind this to not keep the favicon and zimFilePath in Bookmark so i have an alternative approach for this (at the first start or after importing the bookmarks) of the application when the ZIM files are loaded in the library screen then we can add the ZIM files in the library, so that all the imported bookmarks will become valid again. @kelson42 What do you think?

rgaudin commented 1 month ago

You could simplify this by showing a count of invalid bookmarks in the bookmark view: “You have 32 invalid bookmarks (used in books that are not in your library)”. You could even list the related books with links to the catalog manager so user downloads them again. Indeed that sounds like a practical use case

kelson42 commented 1 month ago

@MohitMaliFtechiz Not importing/exporting the library.xml is a clear requirement. It has been expressed many times and I would appreciate that we don't keep discussing it.

kelson42 commented 1 month ago

@MohitMaliFtechiz For the moment we don't want to keep the favicon in bookmarks.xml for space reasons and because we believe it's not necessary.

But Kiwix has somehow to give an insight to "dead" bookmarks.

Bookmarks entries should not save the ZIM path, but the ZIM metadata Id and name. Based on them the libkiwix is not only able to retrieve the book in library, but any new version of the same book.

Closing this issue as therw nothing clear we shozld/could do in libkiwix.