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

[editor] Support disabling of editing when pdfjs.enablePermissions is set (issue 15049) #15075

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

For encrypted PDF documents without the required permissions set, this patch adds support for disabling of Annotation-editing. However, please note that it also requires that the pdfjs.enablePermissions preference is set to true (since PDF document permissions could be seen as user hostile).[1]

As I started looking at the issue, it soon became clear that only trying to fix the issue without slightly re-factor the surrounding code would be somewhat difficult.
The following is an overview of the changes in this patch; sorry about the size/scope of this!

  • Use a new AnnotationEditorUIManager-instance for each PDF document opened in the GENERIC viewer, to prevent user-added Annotations from "leaking" from one document into the next.

  • Re-factor the BaseViewer.#initializePermissions-method, to simplify handling of temporarily disabled modes (e.g. for both Annotation-rendering and Annotation-editing).

  • When editing is enabled, let the Editor-buttons be disabled until the document has loaded. This way we avoid the buttons becoming clickable temporarily, for PDF documents that use permissions.

  • Slightly re-factor how the Editor-buttons are shown/hidden in the viewer, and reset the toolbar-state when a new PDF document is opened.

  • Flip the order of the Editor-buttons and the pre-exising toolbarButtons in the "toolbarViewerRight"-div. (To help reduce the size, a little bit, for the PR that adds new Editor-toolbars.)

  • Enable editing by default in the development viewer, i.e. gulp server, since having to (repeatedly) do that manually becomes annoying after a while.

  • Finally, support disabling of editing when pdfjs.enablePermissions is set; fixes issue Disable editing when the pdf disallows it #15049.


[1] Either manually with about:config, or using e.g. a Group Policy.

@Snuffleupagus Snuffleupagus linked an issue Jun 20, 2022 that may be closed by this pull request
web/app.js Outdated Show resolved Hide resolved
web/base_viewer.js Outdated Show resolved Hide resolved
…is set (issue 15049)

For encrypted PDF documents without the required permissions set, this patch adds support for disabling of Annotation-editing. However, please note that it also requires that the `pdfjs.enablePermissions` preference is set to `true` (since PDF document permissions could be seen as user hostile).[1]

As I started looking at the issue, it soon became clear that *only* trying to fix the issue without slightly re-factor the surrounding code would be somewhat difficult.
The following is an overview of the changes in this patch; sorry about the size/scope of this!

 - Use a new `AnnotationEditorUIManager`-instance *for each* PDF document opened in the GENERIC viewer, to prevent user-added Annotations from "leaking" from one document into the next.

 - Re-factor the `BaseViewer.#initializePermissions`-method, to simplify handling of temporarily disabled modes (e.g. for both Annotation-rendering and Annotation-editing).

 - When editing is enabled, let the Editor-buttons be `disabled` until the document has loaded. This way we avoid the buttons becoming clickable temporarily, for PDF documents that use permissions.

 - Slightly re-factor how the Editor-buttons are shown/hidden in the viewer, and reset the toolbar-state when a new PDF document is opened.

 - Flip the order of the Editor-buttons and the pre-exising toolbarButtons in the "toolbarViewerRight"-div. (To help reduce the size, a little bit, for the PR that adds new Editor-toolbars.)

 - Enable editing by default in the development viewer, i.e. `gulp server`, since having to (repeatedly) do that manually becomes annoying after a while.

 - Finally, support disabling of editing when `pdfjs.enablePermissions` is set; fixes issue 15049.

---

[1] Either manually with `about:config`, or using e.g. a [Group Policy](https://github.com/mozilla/policy-templates).
@Snuffleupagus
Copy link
Collaborator Author

/botio integrationtest

1 similar comment
@calixteman
Copy link
Contributor

/botio integrationtest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/7c46917ec715871/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/99ef7c367842079/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/7c46917ec715871/output.txt

Total script time: 4.53 mins

  • Integration Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/99ef7c367842079/output.txt

Total script time: 7.28 mins

  • Integration Tests: FAILED

@calixteman
Copy link
Contributor

I tested the patch locally with enablePermissions and annotationEditorEnabled set to true with the file in #15049 and the buttons are still enabled. Clicking on one of them is just throwing an exception.

@Snuffleupagus
Copy link
Collaborator Author

I tested the patch locally with enablePermissions and annotationEditorEnabled set to true with the file in #15049 and the buttons are still enabled. Clicking on one of them is just throwing an exception.

Strange, it works for me...

Can you try a skip-cache reload of the viewer, to ensure that the old button state isn't being kept somehow?

@calixteman
Copy link
Contributor

It works with an hard refresh.

Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

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.

Disable editing when the pdf disallows it
3 participants