Skip to content

Conversation

@BrayanDSO
Copy link
Member

@BrayanDSO BrayanDSO commented Apr 9, 2024

Summary of the issue:

Loading files with file:/ scheme in much faster in an Android Webview, and that's what 2.15.6 did by setting the Webview base url to the media directory.

But in 2.16/2.17, in order to enable POST requests to be done, the base url was set to localhost and the files were delivered by the server. After noticing that loading files was slower with the server, a WebViewAssetLoader was used, but it was still slower than file:/. The next alternative was parsing the card's HTML to use file:/ for media sources, but that leads to CORS issues and sometimes HTML parsing issues.

So, here's what I believe that should be the definitive solution, which is to remap POST requests base url, so the Webview base url can be set to the media directory so the 2.15.6 behavior is restored.


Basically, this replaces "/_anki/" in reviewer.js and reviewer_extras_bundle.js so a custom post base url can be used if set in AnkiDroid.

My experience with Rust is almost zero, so I expect that there should be better practices than what I did.

Tested with this PR by adding console.log("foo"); into Custom scheduling deck option.


Ideally, this should be unit tested, but I currently don't have time for that, nor for dealing with requested changes in this PR, so please take over this if time is of the essence.

@david-allison
Copy link
Member

@BrayanDSO needs a reformat via cargo fmt

@dae
Copy link
Contributor

dae commented Apr 10, 2024

Does the rename work on Windows while the file is still open?

Doing it at build time is a valid approach, but AnkiMobile takes the simpler approach of doing the replacement when the webpage is being served to the frontend.

@BrayanDSO
Copy link
Member Author

Does the rename work on Windows while the file is still open?

I think so? When I was trying to do this, I had reviewer.js opened in VS Code and it was updated.

Is there a scenario where the file would be opened while the backend is being built?

Doing it at build time is a valid approach, but AnkiMobile takes the simpler approach of doing the replacement when the webpage is being served to the frontend.

Not sure if that will be simpler in AnkiDroid. I believe that the end result will be the same, so I'll opt for the lazier approach of not having to reimplement this in another way.

@dae
Copy link
Contributor

dae commented Apr 11, 2024

Windows can throw errors when you try to do something like delete a file that is currently open for writing. But if the build scripts are working on a Windows machine for you (and not WSL), then it sounds like it's fine.

Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Good to go. One nice to have

allows using `ankidroid.postBaseUrl` as the base url for POST requests
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Seems functional enough - read through the other comments. Release build is on mac so that won't be a problem in infrastructure, and if it works for the people that say they were working on it, works for me

I'm most interested in making sure that the idea actually works, and if the replacement needs to move later I think that's minor compared to getting the idea of the remap into backend/frontend

@mikehardy
Copy link
Member

I'm going to do the release process as well - that changed so recently and has only been exercised once so I'm keen to see it exercised again, especially since the version bump is already in here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants