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.

Sign up for GitHub

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

Jump to bottom

Saving and Restoring the Web View Navigation History Across Sessions #3996

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft

Saving and Restoring the Web View Navigation History Across Sessions #3996

wants to merge 19 commits into from

Conversation

Saifuddin53
Copy link

@Saifuddin53 Saifuddin53 commented Sep 13, 2024 •
edited
Loading

Fixes #398

Still in development

  • Enhanced history restoration by managing back and forward stacks separately and loading URLs accordingly.
  • The integrated room database is used to fetch and restore page history asynchronously using RxJava.
  • Implemented a mechanism to clear and retrieve history based on database entries.

How It Works:

  • When the reader fragment is paused the history is stored in the room
  • Upon resuming the WebView/ReaderFragment, the history is fetched from the Room database and restored.
  • Backward and forward navigation is re-established based on the saved history.

Screenshots

@Saifuddin53
Copy link
Author

Saifuddin53 commented Sep 19, 2024 •
edited
Loading

Hello @MohitMaliFtechiz @kelson42
I've implemented a history restoration approach for WebView that loads pages sequentially from the back and forward stacks (stored in our room database) when the reader fragment is resumed. This solution is functional, but I would appreciate your feedback on the implementation and suggestions for improvement.

WhatsApp.Video.2024-09-19.at.11.05.05_9d0b7924.mp4

But it has some cons.

  • Currently, the time taken to restore history is directly proportional to the size of the back and forward stacks. Each URL load has a delay of 500ms, which means for a history with many items, the total restoration time could be several seconds. This could negatively impact user experience, particularly if the restoration feels too slow.

  • Ideally, the Reader screen should be blocked with a loading indicator until the history is fully restored to avoid interruptions.

  • The restoration only loads the page URLs but does not retain the user’s scroll position. When the Reader is reopened, users are taken to the top of each page, losing the context of where they left off, which can be frustrating for longer content.

On the bright side, the current approach makes sure everything is restored in order, so the navigation history stays consistent.

Also, I’ve experimented with a few other approaches to reduce the restoration time, but none of them offered the same level of accuracy. It seems like there’s a trade-off between speed and ensuring the history is restored correctly.

I’d appreciate your input on how we might address these challenges and any suggestions you have for further refinement of this approach

@Saifuddin53 Saifuddin53 changed the title Saving the Web View Navigation History Across Sessions Saving and Restoring the Web View Navigation History Across Sessions Sep 19, 2024
@MohitMaliFtechiz
Copy link
Collaborator

@Saifuddin53 Thanks for your PR, and inputs. I will test and review your PR. I have fixed the detekt issue you were facing, Now, you can continue doing your work. The codeFactor is failing can you please fix it?

@kelson42
Copy link
Collaborator

@MohitMaliDeveloper Any news?

MohitMaliFtechiz
MohitMaliFtechiz requested changes Sep 19, 2024
View reviewed changes
Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Saifuddin53 For the restoration timing issue, I am seeing the possible solution. In the meantime can you please make these review changes?

.idea/codeStyles/Project.xml Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/dao/PageHistoryRoomDao.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/dao/entities/PageHistoryRoomEntity.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt Outdated Show resolved Hide resolved
core/src/main/res/values-b+be+tarask/strings.xml Outdated Show resolved Hide resolved
core/src/main/res/values-b+be+tarask+old/strings.xml Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/page/history/adapter/PageHistoryItem.kt Outdated Show resolved Hide resolved
@kelson42
Copy link
Collaborator

kelson42 commented Oct 1, 2024

@Saifuddin53 Any news here?

@Saifuddin53
Copy link
Author

@kelson42 trying to optimize this algo.

MohitMaliFtechiz
MohitMaliFtechiz requested changes Oct 9, 2024
View reviewed changes
Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Saifuddin53 Please do these changes it will reduce the restoring time, and increase the overall performance and code quality.

core/src/main/java/org/kiwix/kiwixmobile/core/di/components/CoreComponent.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/DatabaseModule.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/data/KiwixRoomDatabase.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/page/history/adapter/WebViewHistoryItem.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt Outdated Show resolved Hide resolved
core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt Outdated Show resolved Hide resolved
@MohitMaliDeveloper
Copy link
Contributor

@Saifuddin53 What is the status on this PR? Do you need any help from me in completing the PR?

@Saifuddin53
Copy link
Author

Saifuddin53 commented Oct 16, 2024 •
edited
Loading

@MohitMaliDeveloper, can you please help me with the restoration algorithm? As, when I try to load the pages using the onPageFinished callback, the pages load too quickly, or the asynchronous behavior of the WebView is not properly handled,so it sometimes skips some entries.

@MohitMaliFtechiz
Copy link
Collaborator

@Saifuddin53 I will have a look at this after completing #4040.

@MohitMaliFtechiz
Copy link
Collaborator

@Saifuddin53 The page history is restored when we open a page from a bookmark, history, etc. However, it should not restore the webView page history under these conditions(it should only restore the webView history when the application is restarted or we switch between pages). See the below video:

IssueWhileRestoringWebViewHistory.mp4

As in this condition, it should only open the page on which the user just clicked on the bookmark, history, etc.

Saifuddin53 and others added 14 commits October 25, 2024 16:27
@Saifuddin53 @MohitMaliFtechiz
Saved webbackforwardlist to room
d986d65
@MohitMaliDeveloper @MohitMaliFtechiz
Retrieving page history from room
0b790dd
@Saifuddin53 @MohitMaliFtechiz
Restoring the internal webbackforwardlist
97d99ef
@MohitMaliDeveloper @MohitMaliFtechiz
Fixed the detekt issue
46092f8
@MohitMaliDeveloper @MohitMaliFtechiz
Naming convention improved
a54f210
@Saifuddin53 @MohitMaliFtechiz
Code factor improvements
1daf6e3
@Saifuddin53 @MohitMaliFtechiz
Automated changes reverted
874b7c3
@Saifuddin53 @MohitMaliFtechiz
Automated changes reverted
7b3d8d3
@Saifuddin53 @MohitMaliFtechiz
Automated changes reverted
cd50fb7
@Saifuddin53 @MohitMaliFtechiz
Updated room schema
198fc20
@Saifuddin53 @MohitMaliFtechiz
Naming conventions improved and some reviewed changes
abee437
@MohitMaliDeveloper @MohitMaliFtechiz
Fixed detekt issue.
98b80ca
@MohitMaliDeveloper @MohitMaliFtechiz
Implemented new approach to save and retrieve the webView history.
947c7a9 
* Added new logic to save and restore WebView navigation history directly, without loading pages one by one.
* Refactored Room database schema to support saving the history of all tabs instead of just one.
* Applied changes to custom apps, currently working on integrating and testing with the Kiwix app.
* Improved multi-tab restoration, ensuring history of all tabs is restored.
* Improved the saving of WebView history by saving the list instead of saving each entry one by one, reducing database calls and saving time.
@MohitMaliDeveloper @MohitMaliFtechiz
Fixed: Restore web view history in Kiwix app.
b0750a9 
* Improved restoration of web view history after opening a searched article.
* Refactored `restoreViewStateOnValidJSON` and `restoreViewStateOnInvalidJSON` methods to save and retrieve web view history from the Room database.
* Added detailed comments to methods for better understanding, including guidance for developers to check subclass implementations before making modifications.
* Fixed the `static analysis` errors.
@MohitMaliDeveloper
Fixed: Searched item was not opening in custom apps.
832340f 
* Created a common approach for both Kiwix and custom apps to open the searched item after restoring the tabs, ensuring smooth functionality in both applications and reducing the likelihood of future errors.
@MohitMaliDeveloper
Fixed: FindInPage functionality not working in Kiwix and custom apps.
58724de 
* Established a unified approach for both Kiwix and custom apps to trigger findInPage after restoring tabs, ensuring consistent functionality across both applications and minimizing potential future errors.
@MohitMaliDeveloper
Improved the saving of tabs.
16192b3 
* Since tabs are now saved with proper WebView history on the IO thread, saving takes some time. As a result, when switching to the history, bookmarks, or notes screens, WebView history sometimes wasn't saved properly. To address this, we moved the tab-saving functionality from onPause to when the page is fully loaded in WebView, which ensures proper saving of tabs and reduces the chances of history loss.
* This change fix the restoring the tabs when we switch to the bookmark, history, notes screens.
@MohitMaliDeveloper
Fixed: Reader screen was showing twice when pressing the back button …
bb2a412 
…after opening a page from the history, bookmarks, or notes screens in custom apps.
@MohitMaliDeveloper
Fix: Main page of ZIM file not loading when opening a note in the not…
c6a2769 
…es screen

* Resolved the issue where the main page of the ZIM file was not loading when a note was opened from the notes screen, when a different ZIM file was set in the reader. Now, when opening notes, the application correctly sets the corresponding ZIM file in the reader, ensuring the main page is displayed as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MohitMaliFtechiz MohitMaliFtechiz MohitMaliFtechiz requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Save the back stack of visited pages before exiting from app
4 participants
@Saifuddin53 @MohitMaliFtechiz @kelson42 @MohitMaliDeveloper