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

Error during destruction of the Editing-instance #15149

Closed
Snuffleupagus opened this issue Jul 8, 2022 · 1 comment · Fixed by #15151
Closed

Error during destruction of the Editing-instance #15149

Snuffleupagus opened this issue Jul 8, 2022 · 1 comment · Fixed by #15151
Assignees

Comments

@Snuffleupagus
Copy link
Collaborator

Attach (recommended) or Link to PDF file here: N/A

Configuration:

Steps to reproduce the problem:

  1. Open the development viewer locally, after running gulp server, http://localhost:8888/web/viewer.html
  2. Enable FreeText editing. (Note that Ink editing is similarly affected.)
  3. Click on any page, and add a new comment (e.g. "Hello world").
  4. Open the web-console, and run PDFViewerApplication.close().

What is the expected behavior? (add screenshot)
Closing works correctly.

What went wrong? (add screenshot)
An error is thrown:

Uncaught (in promise) TypeError: editor.destroy is not a function
    destroy http://localhost:8888/src/display/editor/tools.js:439
    setDocument http://localhost:8888/web/base_viewer.js:644
    close http://localhost:8888/web/app.js:866
    async* debugger eval code:1

Basically, the issue is that none of the AnnotationEditor-classes implement a destroy-method. The simplest solution would be to just remove this code:

for (const editor of this.#allEditors.values()) {
editor.destroy();
}

Another solution would be to actually implement destroy-functionality for the AnnotationEditor-classes, but I'm not sure if that's actually necessary here (given that we're tearing down the current Editing-instance)?

/cc @calixteman What solution is "correct" here? If it's the first one I could definitely submit a PR to fix it.

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension): N/A

@calixteman
Copy link
Contributor

It's a leftover: we just need to remove these 3 lines.

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

Successfully merging a pull request may close this issue.

2 participants