-
-
Notifications
You must be signed in to change notification settings - Fork 29
Remap POST requests url #367
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
Conversation
|
@BrayanDSO needs a reformat via |
|
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. |
I think so? When I was trying to do this, I had Is there a scenario where the file would be opened while the backend is being built?
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. |
|
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. |
david-allison
left a comment
There was a problem hiding this 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
mikehardy
left a comment
There was a problem hiding this 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
|
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 |
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
WebViewAssetLoaderwas used, but it was still slower thanfile:/. The next alternative was parsing the card's HTML to usefile:/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/"inreviewer.jsandreviewer_extras_bundle.jsso 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");intoCustom schedulingdeck 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.