Skip to content

Load local file requested using file protocol in reviewer (json & text file)#11564

Closed
krmanik wants to merge 1 commit into
ankidroid:mainfrom
krmanik:load-local-file
Closed

Load local file requested using file protocol in reviewer (json & text file)#11564
krmanik wants to merge 1 commit into
ankidroid:mainfrom
krmanik:load-local-file

Conversation

@krmanik

@krmanik krmanik commented Jun 7, 2022

Copy link
Copy Markdown
Member

Pull Request template

Purpose / Description

I use Anki-Xiehanzi deck to study Chinese language and always want it to be offline, but json files in the deck need to be served over localhost because file:/// protocol gives error when loading file other than js.

Anki desktop load this file because it uses localhost so all files in collection.media are accessible over http://127.0.0.1:<port>.

Fixes

Fixes #8525

Approach

When file (json & text) requested over file:/// protocol then implementation in this PR will return the web response with file content.

  1. Used idea from Porting card info krmanik/Anki-Android#23 for serving file over file:///
  2. Create a class for handling mime type and web response
  3. Check if url start with file protocol and ends with file extension
  4. Then from AbstractFlashcardViewer decoded url passed to AnkiLoadLocalFile
  5. In AnkiLoadLocalFile the url split with / and last string used as filename
  6. The filename and mime type used to read file and return response

How Has This Been Tested?

  1. Created a test.json file in collection.media dir
  2. Used chrome dev tools to send request
var xhttp = new XMLHttpRequest();
xhttp.onreadystatechange = function() {
    if (this.readyState == 4 && this.status == 200) {
       console.log(xhttp.responseText);
    }
};
xhttp.open("GET", "test.json", true);
xhttp.send();

Note: fetch will not return the file, but XMLHttpRequest will get the file.

Fetch API cannot load file:///storage/emulated/0/AnkiDroid/collection.media/test.json. URL scheme "file" is not supported.

image

Learning (optional, can help others)

krmanik#23

Checklist

Please, go through these checks before submitting the PR.

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@david-allison

Copy link
Copy Markdown
Member

Have you looked at the issues surrounding #10213

@dae

dae commented Jun 8, 2022

Copy link
Copy Markdown
Contributor

One approach worth considering in the future is switching mViewerUrl away from a file:// scheme, and using a fake https URL instead. Android appears to provide a helper class to make it easier to serve file content from shouldInterceptRequest: https://developer.android.com/reference/androidx/webkit/WebViewAssetLoader

@krmanik

krmanik commented Jun 8, 2022

Copy link
Copy Markdown
Member Author

Have you looked at the issues surrounding #10213

I have tested by adding following card template and it seems, it is working.

<link rel="stylesheet" href="tmp.css"/>
<script src="tmp.js"></script>
<img src="tmp.png"></img>

One approach worth considering in the future is switching mViewerUrl away from a file:// scheme, and using a fake https URL instead. Android appears to provide a helper class to make it easier to serve file content from shouldInterceptRequest: https://developer.android.com/reference/androidx/webkit/WebViewAssetLoader

I created a issues, and implement the suggested option in next PR.

@github-actions

github-actions Bot commented Aug 7, 2022

Copy link
Copy Markdown
Contributor

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

@github-actions github-actions Bot added the Stale label Aug 7, 2022
@krmanik krmanik removed the Stale label Aug 12, 2022
@mikehardy

Copy link
Copy Markdown
Member

No pressure at all here @krmanik - I'm just going through the whole PR queue oldest to newest and I'm curious what the plan here is? I have looked in your repo with you at PR 23 over there, and I know you've got the TS files working in the backend (but maybe we need a change there?). Let me know what you need to move this forward and I'll do my best to collaborate.

In the meantime, I'm catching up again and now that summer travel is over I hope to get through the JS add-on work you've done

@mikehardy mikehardy added the Needs Author Reply Waiting for a reply from the original author label Aug 19, 2022
@krmanik

krmanik commented Aug 19, 2022

Copy link
Copy Markdown
Member Author

Hi,
I think the current PR should be wait. After merge of import csv I will continue on this.
The changes needs to be done on reviewer so this PR can be reworked later.

If this issue fixed #11570 then the current PR will be closed. So, it needs work on the issues.

@github-actions

github-actions Bot commented Dec 2, 2022

Copy link
Copy Markdown
Contributor

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

@github-actions github-actions Bot added the Stale label Dec 2, 2022
@mikehardy

Copy link
Copy Markdown
Member

scoped storage will finish, one of these days, and I can hopefully turn attention back to all the javascript work + crowdin API updates

@mikehardy mikehardy added Keep Open avoids the stale bot and removed Stale labels Dec 3, 2022
@nihil-admirari

Copy link
Copy Markdown
Contributor

There is an already existing undocumented facility for loading files from the media folder: #7764 (comment). It works on Android 10+ as is; older Androids require a fix for correct MIME type identification (critical for JS modules, but not necessarily critical for a fetch).

Wouldn't the following be already sufficient for JSON fetching:

const mediaRoot = globalThis.AnkiDroidJS ? 'https://appassets.androidplatform.net'
                                         : globalThis.ankiPlatform === 'desktop' ? '' : '.';
await fetch(`${mediaRoot}/file.json`);

@krmanik

krmanik commented Nov 4, 2023

Copy link
Copy Markdown
Member Author

This PR can be closed now because latest version of AnkiDroid contains http server which can be used to fetch the json/files from collection.media.

@krmanik krmanik closed this Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Keep Open avoids the stale bot Needs Author Reply Waiting for a reply from the original author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding read json and write json function to JS API

5 participants