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.

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

Storage location not created and setting not honored #1053

Open
VA1DER opened this issue Apr 14, 2022 · 4 comments · May be fixed by #1073
Open

Storage location not created and setting not honored #1053

VA1DER opened this issue Apr 14, 2022 · 4 comments · May be fixed by #1073

Comments

@VA1DER
Copy link

VA1DER commented Apr 14, 2022

On initial installation, the output storage location defaults to /storage/emulated/0/PDFfiles

This directory is not, however, created and any attempts at saving output will claim to succeed but won't. If the directory is created, however, saving output will succeed.

If the storage location is changed it will make no difference, the app will always only try the PDFfiles directory and always silently fail if that directory doesn't exist.

@codegsaini
Copy link
Contributor

What you want to say is,

Case 1) App display directory path as /storage/emulated/0/PDFfiles in the settings and doesn't even create such directory there.
And When it try to save output file there, then it show success message but fail to do so internally.
But when we manually create the directory, then it works as expected.

Case 2) When we change location path from settings then app still trying to save output file in PDFfiles directory.

Right?

I will look into it soon... until then anyone can consider working on this issue :)

@KOISHI-CHEN
Copy link

KOISHI-CHEN commented May 7, 2022

@codegsaini @VA1DER Hi, I would like work this, but could you give more examples how this problem happen? Does this error occur in some spefic functions or Is this mistake associated with all of the storage location

@KOISHI-CHEN
Copy link

@codegsaini Hi, it seems like I have find what the problem is.
First is about the path "/storage/emulated/0/PDFfiles" mentioned in the problem.
The only one place I find in the whole project which is related to that path is this code fragment in PdfToImageFragment.java

    @OnClick(R.id.viewImagesInGallery)
    void onImagesInGalleryClick() {
        Intent intent = new Intent();
        intent.setAction(Intent.ACTION_VIEW);
        Uri imagesUri = Uri.parse("content:///storage/emulated/0/PDFfiles/");
        intent.setDataAndType(imagesUri, "image/*");
        intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
        startActivity(intent);
    }

Although it is named as viewImagesInGallery, it just request some external app to view the image. And the direction is never created.
And I also find two other related problem in the project
One is about it will actually not create the default storage path after installing, the default storage path is created only when after someone execute getDefaultStoragelocation() which is a method in String.utils. And some functions like PdfToImages will not invoke this mehod or they will just create the path arbitrary. So if user first use the PdfToImages, something wrong with path happen. And the storage of different function is ridiculously different.

Second is that I find in the settings the default storage path is fixed. And in English version it is marked as non-changeable. But In Chinese version and or other languages (also find in ja), it is marked as change storage location.

So above all, I considerate that I can first revise the .viewImagesInGallery(). Then encapsulate the method about storage to some storage utils so that everyone will not create some storage path themselves. And I will also create the default storage path immediately after installing. Finally, I will try to allow the storage path to be changed by User.

Is it ok? @codegsaini

@codegsaini
Copy link
Contributor

create the default storage path immediately after installing

allow the storage path to be changed by User.

@KOISHI-CHEN These two changes may solve the issue. Consider these ....

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 a pull request may close this issue.

3 participants