From bed1a1baa19e9f0f63bdb637d7c02aa3dd38a4d8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 9 Dec 2022 23:24:18 +0100 Subject: [PATCH] [AnnotationEditorLayerBuilder] Inline the `destroy` code in the `cancel` method It doesn't seem necessary to have a *separate* `destroy` method given that the `cancel` method always invokes it unconditionally. In the `PDFPageView.reset` method we currently attempt to call `destroy` directly, however that'll never actually happen since either: - We're keeping the annotationEditorLayer, in which case we're just hiding the layer and nothing more (and the relevant branch is never entered). - We're removing the annotationEditorLayer, in which case the `PDFPageView.cancelRendering` method has already cancelled *and* nulled it (and there's thus nothing left to `destroy` here). *Please note:* Hopefully I'm not overlooking something obvious here, since both reading through the code *and* also adding `console.log(this.annotationEditorLayer);` [before this line](https://github.com/mozilla/pdf.js/blob/9d4aadbf7a32c8a994818e848fb2ec883f257702/web/pdf_page_view.js#L438) suggests that it's indeed unnecessary. --- web/annotation_editor_layer_builder.js | 17 +++++++---------- web/pdf_page_view.js | 4 ---- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/web/annotation_editor_layer_builder.js b/web/annotation_editor_layer_builder.js index e912ff60b..8a2cb7853 100644 --- a/web/annotation_editor_layer_builder.js +++ b/web/annotation_editor_layer_builder.js @@ -98,7 +98,13 @@ class AnnotationEditorLayerBuilder { cancel() { this._cancelled = true; - this.destroy(); + + if (!this.div) { + return; + } + this.pageDiv = null; + this.annotationEditorLayer.destroy(); + this.div.remove(); } hide() { @@ -114,15 +120,6 @@ class AnnotationEditorLayerBuilder { } this.div.hidden = false; } - - destroy() { - if (!this.div) { - return; - } - this.pageDiv = null; - this.annotationEditorLayer.destroy(); - this.div.remove(); - } } export { AnnotationEditorLayerBuilder }; diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index befa9de82..046e09963 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -431,18 +431,14 @@ class PDFPageView { // so they are not displayed on the already resized page. this.annotationLayer.hide(); } - if (annotationEditorLayerNode) { this.annotationEditorLayer.hide(); - } else { - this.annotationEditorLayer?.destroy(); } if (xfaLayerNode) { // Hide the XFA layer until all elements are resized // so they are not displayed on the already resized page. this.xfaLayer.hide(); } - if (textLayerNode) { this.textLayer.hide(); }