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

Fix disabled annotation edition in view only files #1108

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Jan 11, 2025

When a PDF file was shared without edit permissions the annotation editor buttons were hidden in the toolbar. Besides being a dirty trick, it only prevented creating new annotations, but not editing existing ones. Now the parameter annotationEditorMode of PDF.js set to AnnotationEditorType.DISABLE is used instead, which properly disables editing annotations and also takes care of removing the buttons from the toolbar if needed.

Note that the editor mode separator is no longer hidden; it was hidden back in the day when edition was not supported yet in the PDF viewer, and once edition was added it should have been shown whenever the editor buttons were shown, but it was not the case. Now it is shown by default and hidden as needed by PDF.js itself when annotation edition is disabled.

How to test

  • Open the Files app
  • Upload a PDF file
  • Open the PDF file and add a text annotation
  • Add a public share for the PDF file (which, by default, will be view only)
  • In a private window, open the public share
  • Double click on the text annotation

Result with this pull request

The text annotation does not enter in edition mode; in the browser console the error The AnnotationEditor is not enabled. will be printed

Result without this pull request

The text annotation enters in edition mode; a pop up is shown in the toolbar at the place where the text annotation button would be if it was visible

@danxuliu danxuliu added this to the Nextcloud 31 milestone Jan 11, 2025
@danxuliu danxuliu force-pushed the fix-disabled-annotation-edition-in-view-only-files branch 2 times, most recently from 1537d03 to 640758c Compare January 20, 2025 10:57
@danxuliu
Copy link
Member Author

/compile amend /

@nextcloud-command nextcloud-command force-pushed the fix-disabled-annotation-edition-in-view-only-files branch from 640758c to 6a3d1fa Compare January 20, 2025 10:59
@danxuliu danxuliu requested a review from szaimen January 20, 2025 11:17
@blizzz blizzz modified the milestones: Nextcloud 31, Nextcloud 32 Jan 29, 2025
@danxuliu danxuliu force-pushed the fix-disabled-annotation-edition-in-view-only-files branch from 6a3d1fa to 57842d7 Compare January 29, 2025 23:07
When a PDF file was shared without edit permissions the annotation
editor buttons were hidden in the toolbar. Besides being a dirty trick,
it only prevented creating new annotations, but not editing existing
ones. Now the parameter "annotationEditorMode" of PDF.js set to
"AnnotationEditorType.DISABLE" is used instead, which properly disables
editing annotations and also takes care of removing the buttons from the
toolbar if needed.

Note that the editor mode separator is no longer hidden; it was hidden
back in the day when edition was not supported yet in the PDF viewer,
and once edition was added it should have been shown whenever the editor
buttons were shown, but it was not the case. Now it is shown by default
and hidden as needed by PDF.js itself when annotation edition is
disabled.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu force-pushed the fix-disabled-annotation-edition-in-view-only-files branch from 57842d7 to 5b966a6 Compare January 29, 2025 23:14
@danxuliu danxuliu marked this pull request as ready for review January 29, 2025 23:18
@danxuliu
Copy link
Member Author

/backport to stable31

@danxuliu
Copy link
Member Author

/backport to stable30

@danxuliu
Copy link
Member Author

/backport to stable29

@danxuliu danxuliu enabled auto-merge January 29, 2025 23:18
Copy link
Collaborator

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Tested and works :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants