From 050093c9f519154f26f3d5757ffc7290fd98ba8f Mon Sep 17 00:00:00 2001
From: Calixte Denizet <calixte.denizet@gmail.com>
Date: Fri, 22 Sep 2023 09:43:19 +0200
Subject: [PATCH] [Editor] Tweak the save flow in the alt-text dialog

When the user edit an existing alt-text and remove it, we want to be able
to save this state and consequently remove the done state from the
alt-text button.
Remove the button from its parent when the editor is removed: it should
help to save few Kb of memory.
---
 src/display/editor/editor.js | 31 ++++++++++++++++++++++---------
 web/alt_text_manager.js      | 10 ++++++++--
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js
index 094810577..2898b293a 100644
--- a/src/display/editor/editor.js
+++ b/src/display/editor/editor.js
@@ -822,18 +822,17 @@ class AnnotationEditor {
     this.fixAndSetPosition();
   }
 
-  addAltTextButton() {
+  async addAltTextButton() {
     if (this.#altTextButton) {
       return;
     }
     const altText = (this.#altTextButton = document.createElement("button"));
     altText.className = "altText";
-    AnnotationEditor._l10nPromise
-      .get("editor_alt_text_button_label")
-      .then(msg => {
-        altText.textContent = msg;
-        altText.setAttribute("aria-label", msg);
-      });
+    const msg = await AnnotationEditor._l10nPromise.get(
+      "editor_alt_text_button_label"
+    );
+    altText.textContent = msg;
+    altText.setAttribute("aria-label", msg);
     altText.tabIndex = "0";
     altText.addEventListener(
       "click",
@@ -864,7 +863,12 @@ class AnnotationEditor {
 
   async #setAltTextButtonState() {
     const button = this.#altTextButton;
-    if (!button || (!this.#altTextDecorative && !this.#altText)) {
+    if (!button) {
+      return;
+    }
+    if (!this.#altText && !this.#altTextDecorative) {
+      button.classList.remove("done");
+      this.#altTextTooltip?.remove();
       return;
     }
     AnnotationEditor._l10nPromise
@@ -879,7 +883,6 @@ class AnnotationEditor {
       tooltip.className = "tooltip";
       tooltip.setAttribute("role", "tooltip");
       const id = (tooltip.id = `alt-text-tooltip-${this.id}`);
-      button.append(tooltip);
       button.setAttribute("aria-describedby", id);
 
       const DELAY_TO_SHOW_TOOLTIP = 100;
@@ -911,6 +914,10 @@ class AnnotationEditor {
           "editor_alt_text_decorative_tooltip"
         )
       : this.#altText;
+
+    if (!tooltip.parentNode) {
+      button.append(tooltip);
+    }
   }
 
   getClientDimensions() {
@@ -1238,6 +1245,12 @@ class AnnotationEditor {
     } else {
       this._uiManager.removeEditor(this);
     }
+
+    // The editor is removed so we can remove the alt text button and if it's
+    // restored then it's up to the subclass to add it back.
+    this.#altTextButton?.remove();
+    this.#altTextButton = null;
+    this.#altTextTooltip = null;
   }
 
   /**
diff --git a/web/alt_text_manager.js b/web/alt_text_manager.js
index 2687fa5bf..0975ca0df 100644
--- a/web/alt_text_manager.js
+++ b/web/alt_text_manager.js
@@ -52,6 +52,8 @@ class AltTextManager {
 
   #container;
 
+  #previousDecorative = null;
+
   constructor(
     {
       dialog,
@@ -149,6 +151,7 @@ class AltTextManager {
       this.#optionDescription.checked = true;
     }
     this.#previousAltText = this.#textarea.value = altText?.trim() || "";
+    this.#previousDecorative = decorative;
     this.#updateUIState();
 
     this.#currentEditor = editor;
@@ -265,11 +268,14 @@ class AltTextManager {
   }
 
   #updateUIState() {
-    const hasAltText = !!this.#textarea.value.trim();
+    const altText = this.#textarea.value.trim();
     const decorative = this.#optionDecorative.checked;
 
     this.#textarea.disabled = decorative;
-    this.#saveButton.disabled = !decorative && !hasAltText;
+    this.#saveButton.disabled = !(
+      this.#previousDecorative !== decorative ||
+      this.#previousAltText !== altText
+    );
   }
 
   #save() {