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 the bug where button_states() returns None when /AP points to an indirect object. #2845

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

airgalss
Copy link
Contributor

Description: Some PDF widget objects may not have their /AP(Appearance Dictionary) directly pointing to a dictionary, which is enclosed with '<< >>', but rather pointing to an indirect object, expressed by an xref number in the format 'NNN 0 R'. Therefore, the current button_states() only can correctly handle cases where /AP points to a dictionary and cannot handle cases where it points to an indirect object. Consequently, I have introduced additional logic to handle the latter scenario, ensuring that button_states() can accurately return the on/off state names for button widgets even when /AP points to an indirect object.

Test: I have tested the modified button_states() function on the mentioned type of PDF, and it now correctly returns the states instead of None.

Copy link
Contributor

github-actions bot commented Nov 28, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@airgalss
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@airgalss
Copy link
Contributor Author

recheck

@julian-smith-artifex-com
Copy link
Collaborator

Thanks for this. Unfortunately there are some failures in our Test quick action as you can see above, because of mixing tabs and spaces in the new python code.

It would be more useful to us if you made these changes to the rebased implementation in PyMuPDF/src/__init__.py instead of the classic implementation in PyMuPDF/fitz/ - would you be able to do that? The Python code is basically the same so it should be easy enough to port.

@JorjMcKie
Copy link
Collaborator

recheck

There are lines in def butten_states() method that use tab characters instead of 4 spaces.
I would simply change that and copy the method's code to the same-named method in src/__init__.py.

…to an indirect object.

Description: Some PDF widget objects may not have their `/AP`(Appearance Dictionary) directly pointing to a dictionary, which is enclosed with '<< >>', but rather pointing to an indirect object, expressed by an xref number in the format 'NNN 0 R'. Therefore, the current button_states() only can correctly handle cases where `/AP` points to a dictionary and cannot handle cases where it points to an indirect object. Consequently, I have introduced additional logic to handle the latter scenario, ensuring that `button_states()` can accurately return the on/off state names for button widgets even when `/AP` points to an indirect object.
Test: I have tested the modified `button_states()` function on the mentioned type of PDF, and it now correctly returns the states instead of None. I have copied the method's code to the same-named method in `src/__init__.py`.
@airgalss
Copy link
Contributor Author

I have addressed the issue of mixing tabs and spaces, and copied it to src/__init__.py. I hope it should be fine now.

@JorjMcKie JorjMcKie self-requested a review December 12, 2023 13:04
@JorjMcKie JorjMcKie merged commit 50ce442 into pymupdf:main Dec 15, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants