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

Fixed App error on pdf to image for protected files #739

Merged
merged 6 commits into from May 1, 2019

Conversation

jddeep
Copy link
Contributor

@jddeep jddeep commented Apr 27, 2019

Description

Fixed App error on pdf to image for protected files.

Fixes #727

Type of change

Just put an x in the [] which are valid.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

  • ./gradlew assembleDebug assembleRelease
  • ./gradlew checkstyle

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@codeclimate
Copy link

codeclimate bot commented Apr 27, 2019

Code Climate has analyzed commit 9a35d11 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 1

View more on Code Climate.

@jddeep
Copy link
Contributor Author

jddeep commented Apr 27, 2019

@Swati4star @vyankatesh24 Please review.

@sidhuparas
Copy link

@jddeep I couldn't replicate the issue despite trying many times on a password-protected PDF file. I am getting SecurityException: incorrect password... in logcat but the app isn't crashing and snackbar shows that no image files were found.

@jddeep
Copy link
Contributor Author

jddeep commented Apr 27, 2019

@jddeep I couldn't replicate the issue despite trying many times on a password-protected PDF file. I am getting SecurityException: incorrect password... in logcat but the app isn't crashing and snackbar shows that no image files were found.

Yes @sidhuparas The app won't crash. Here, there can be two reasons why you are facing this error. Either your selected pdf has no images in it or the password you entered to decrypt the file is wrong(likely this is the problem).

@jddeep
Copy link
Contributor Author

jddeep commented Apr 28, 2019

@Swati4star Done!

@jddeep
Copy link
Contributor Author

jddeep commented Apr 28, 2019

@vyankatesh24 Some pdfs i am able to convert to images while some are showing similar problem. Can you please test into it and suggest what could be the problem now.

@sidhuparas
Copy link

@jddeep It didn't ask me for password to decrypt. But if it isn't able to decrypt PDF, how can it scan for images?

@jddeep
Copy link
Contributor Author

jddeep commented Apr 28, 2019

@sidhuparas It would ask for password. Please check again. Pull the latest commit and try again.

@sidhuparas
Copy link

@jddeep Actually I didn't even try this PR's code. Master branch code is working fine for me without any crash. Tried for several different files and tried many times.

@jddeep
Copy link
Contributor Author

jddeep commented Apr 28, 2019

@sidhuparas i had the same confusion but @vyankatesh24 pointed it out. Try to extract images from password protected files. It would show error snackbar.

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 I have tested it. The password dialog only appears for encrypted files and not for all files. Have you tested my last and latest changes?

@vyankatesh24
Copy link
Contributor

@vyankatesh24 I have tested it. The password dialog only appears for encrypted files and not for all files. Have you tested my last and latest changes?

Yeah, I had tested the latest changes only

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 I have tested it. The password dialog only appears for encrypted files and not for all files. Have you tested my last and latest changes?

Yeah, I had tested the latest changes only

@vyankatesh24 First try with an unencrypted file. (First test) Then try with encrypted files and see. Please tell me after that.

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 Fixed!

@vyankatesh24
Copy link
Contributor

vyankatesh24 commented May 1, 2019

@vyankatesh24 Fixed!

Still, There are in PdfToImagesFragment.java

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 Where?

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 Everything is fixed. Please review.

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 Kindly please check into the issue label. It took quite a lot of work and time.

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 Changes have been made. Please review.

@jddeep
Copy link
Contributor Author

jddeep commented May 1, 2019

@vyankatesh24 Have a look now.

Copy link
Contributor

@vyankatesh24 vyankatesh24 left a comment

Choose a reason for hiding this comment

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

Well Done @jddeep 👍

@vyankatesh24 vyankatesh24 merged commit 98e1e4a into Swati4star:master May 1, 2019
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.

The app crashed when I tried to convert a password protected PDF file to Image
4 participants