mozilla/pdf.js

changeParent passes string to addDeletedAnnotationElement, breaking deletion tracking on cross‑page moves

Summary

  • Context: The changeParent() method in the annotation editor layer handles moving PDF annotations between pages during drag-and-drop operations.

  • Bug: The method passes a string ID (editor.annotationElementId) to addDeletedAnnotationElement() instead of the expected editor object.

  • Actual vs. expected: The deletion tracking receives a string primitive instead of an editor object, causing undefinedto be added to the deletion tracking set and preventing the deleted flag from being set.

  • Impact: When an existing PDF annotation is moved between pages, the original annotation is not properly marked as deleted, potentially causing it to appear in both locations when the PDF is saved.

Code with bug

In src/display/editor/annotation_editor_layer.js:

// changeParent() method
if (editor.parent && editor.annotationElementId) {
  this.#uiManager.addDeletedAnnotationElement(
    editor.annotationElementId,
  ); // <-- BUG 🔴 passes string ID instead of editor object

  AnnotationEditor.deleteAnnotationElement(editor);
  editor.annotationElementId = null;
}

The method signature in src/shared/tools.js shows the expected parameter type:

/**
 * The annotation element with the given id has been deleted.
 * @param {AnnotationEditor} editor
 */
addDeletedAnnotationElement(editor) {
  this.#deletedAnnotationsElementIds.add(
    editor.annotationElementId,
  );

  this.addChangedExistingAnnotation(editor);
  editor.deleted = true;
}

When a string is passed instead of an editor object, editor.annotationElementId evaluates to undefined, the change tracking receives an invalid key, and the assignment to editor.deleted has no effect on the string primitive.

Codebase inconsistency

The same file contains the correct usage pattern in the detach() method:

// detach() method in annotation_editor_layer.js
if (!this.#isDisabling && editor.annotationElementId) {
  this.#uiManager.addDeletedAnnotationElement(editor);
  // Correctly passes editor object
}

This inconsistency demonstrates that the correct parameter type is known and used elsewhere in the same file, confirming that the bug is a deviation from the established pattern rather than a design decision.

Recommended fix

Change the buggy line to pass the editor object:

this.#uiManager.addDeletedAnnotationElement(editor);  // <-- FIX 🟢

This makes the call consistent with the usage in detach() and matches the documented parameter type.