From 6c005eabb3fbf9e7b4539bb5dcb61a0b3377d084 Mon Sep 17 00:00:00 2001 From: calixteman Date: Wed, 7 Aug 2024 23:11:31 +0200 Subject: [PATCH] Revert "[Editor] Dispatch changes in prefs enableAltTextModelDownload and enableGuessAltText to the viewer (bug 1912024)" --- src/display/editor/stamp.js | 26 +++------- web/app.js | 4 +- web/app_options.js | 4 +- web/firefoxcom.js | 94 +++++-------------------------------- web/genericcom.js | 4 -- web/new_alt_text_manager.js | 81 +++++++++++--------------------- 6 files changed, 51 insertions(+), 162 deletions(-) diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index fc15ea7270b79..0381f475651ed 100644 --- a/src/display/editor/stamp.js +++ b/src/display/editor/stamp.js @@ -141,11 +141,9 @@ class StampEditor extends AnnotationEditor { this._uiManager.useNewAltTextFlow && this.#bitmap ) { - try { - // The alt-text dialog isn't opened but we still want to guess the alt - // text. - this.mlGuessAltText(); - } catch {} + // The alt-text dialog isn't opened but we still want to guess the alt + // text. + this.mlGuessAltText(); } this.div.focus(); @@ -157,11 +155,8 @@ class StampEditor extends AnnotationEditor { } const { mlManager } = this._uiManager; - if (!mlManager) { - throw new Error("No ML."); - } - if (!(await mlManager.isEnabledFor("altText"))) { - throw new Error("ML isn't enabled for alt text."); + if (!mlManager || !(await mlManager.isEnabledFor("altText"))) { + return null; } const { data, width, height } = imageData || @@ -175,18 +170,9 @@ class StampEditor extends AnnotationEditor { channels: data.length / (width * height), }, }); - if (!response) { - throw new Error("No response from the AI service."); - } - if (response.error) { - throw new Error("Error from the AI service."); - } - if (response.cancel) { + if (!response || response.error || !response.output) { return null; } - if (!response.output) { - throw new Error("No valid response from the AI service."); - } const altText = response.output; await this.setGuessedAltText(altText); if (updateAltTextData && !this.hasAltTextData()) { diff --git a/web/app.js b/web/app.js index 684f543334fce..93b07735b3943 100644 --- a/web/app.js +++ b/web/app.js @@ -404,7 +404,9 @@ const PDFViewerApplication = { } else { eventBus = new EventBus(); } - this.mlManager?.setEventBus(eventBus, this._globalAbortController.signal); + if (this.mlManager) { + this.mlManager.eventBus = eventBus; + } this.eventBus = eventBus; this.overlayManager = new OverlayManager(); diff --git a/web/app_options.js b/web/app_options.js index 53c4b5325dcc9..040a08221be7c 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -193,12 +193,12 @@ const defaultOptions = { enableAltTextModelDownload: { /** @type {boolean} */ value: true, - kind: OptionKind.VIEWER + OptionKind.PREFERENCE + OptionKind.EVENT_DISPATCH, + kind: OptionKind.VIEWER + OptionKind.PREFERENCE, }, enableGuessAltText: { /** @type {boolean} */ value: true, - kind: OptionKind.VIEWER + OptionKind.PREFERENCE + OptionKind.EVENT_DISPATCH, + kind: OptionKind.VIEWER + OptionKind.PREFERENCE, }, enableHighlightEditor: { // We'll probably want to make some experiments before enabling this diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 5329886adc879..0993e7070e4ff 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -307,15 +307,11 @@ class FirefoxScripting { } class MLManager { - #abortSignal = null; - #enabled = null; - #eventBus = null; - #ready = null; - #requestResolvers = null; + eventBus = null; hasProgress = false; @@ -334,32 +330,6 @@ class MLManager { this.enableGuessAltText = enableGuessAltText; } - setEventBus(eventBus, abortSignal) { - this.#eventBus = eventBus; - this.#abortSignal = abortSignal; - eventBus._on( - "enablealttextmodeldownload", - ({ value }) => { - if (this.enableAltTextModelDownload === value) { - return; - } - if (value) { - this.downloadModel("altText"); - } else { - this.deleteModel("altText"); - } - }, - abortSignal - ); - eventBus._on( - "enableguessalttext", - ({ value }) => { - this.toggleService("altText", value); - }, - abortSignal - ); - } - async isEnabledFor(name) { return this.enableGuessAltText && !!(await this.#enabled?.get(name)); } @@ -369,17 +339,16 @@ class MLManager { } async deleteModel(name) { - if (name !== "altText" || !this.enableAltTextModelDownload) { + if (name !== "altText") { return; } this.enableAltTextModelDownload = false; this.#ready?.delete(name); this.#enabled?.delete(name); - await this.toggleService("altText", false); - await FirefoxCom.requestAsync( - "mlDelete", - MLManager.#AI_ALT_TEXT_MODEL_NAME - ); + await Promise.all([ + this.toggleService("altText", false), + FirefoxCom.requestAsync("mlDelete", MLManager.#AI_ALT_TEXT_MODEL_NAME), + ]); } async loadModel(name) { @@ -389,7 +358,7 @@ class MLManager { } async downloadModel(name) { - if (name !== "altText" || this.enableAltTextModelDownload) { + if (name !== "altText") { return null; } this.enableAltTextModelDownload = true; @@ -400,53 +369,18 @@ class MLManager { if (data?.name !== "altText") { return null; } - const resolvers = (this.#requestResolvers ||= new Set()); - const resolver = Promise.withResolvers(); - resolvers.add(resolver); - data.service = MLManager.#AI_ALT_TEXT_MODEL_NAME; - const requestPromise = FirefoxCom.requestAsync("mlGuess", data); - - requestPromise - .then(response => { - if (resolvers.has(resolver)) { - resolver.resolve(response); - resolvers.delete(resolver); - } - }) - .catch(reason => { - if (resolvers.has(resolver)) { - resolver.reject(reason); - resolvers.delete(resolver); - } - }); - - return resolver.promise; - } - - async #cancelAllRequests() { - if (!this.#requestResolvers) { - return; - } - for (const resolver of this.#requestResolvers) { - resolver.resolve({ cancel: true }); - } - this.#requestResolvers.clear(); - this.#requestResolvers = null; + return FirefoxCom.requestAsync("mlGuess", data); } async toggleService(name, enabled) { - if (name !== "altText" || this.enableGuessAltText === enabled) { + if (name !== "altText") { return; } this.enableGuessAltText = enabled; - if (enabled) { - if (this.enableAltTextModelDownload) { - await this.#loadAltTextEngine(false); - } - } else { - this.#cancelAllRequests(); + if (enabled && this.enableAltTextModelDownload) { + await this.#loadAltTextEngine(false); } } @@ -469,7 +403,7 @@ class MLManager { if (listenToProgress) { this.hasProgress = true; const callback = ({ detail }) => { - this.#eventBus.dispatch("loadaiengineprogress", { + this.eventBus.dispatch("loadaiengineprogress", { source: this, detail, }); @@ -478,9 +412,7 @@ class MLManager { window.removeEventListener("loadAIEngineProgress", callback); } }; - window.addEventListener("loadAIEngineProgress", callback, { - signal: this.#abortSignal, - }); + window.addEventListener("loadAIEngineProgress", callback); promise.then(ok => { if (!ok) { this.hasProgress = false; diff --git a/web/genericcom.js b/web/genericcom.js index 9bddcbc456eb5..4abde7e8ad980 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -79,10 +79,6 @@ class FakeMLManager { this.enableAltTextModelDownload = enableAltTextModelDownload; } - setEventBus(eventBus, abortSignal) { - this.eventBus = eventBus; - } - async isEnabledFor(_name) { return this.enableGuessAltText; } diff --git a/web/new_alt_text_manager.js b/web/new_alt_text_manager.js index 58d522b3bc8a3..a89ac4575b054 100644 --- a/web/new_alt_text_manager.js +++ b/web/new_alt_text_manager.js @@ -137,10 +137,6 @@ class NewAltTextManager { this.#toggleDisclaimer(); }); - eventBus._on("enableguessalttext", ({ value }) => { - this.#toggleGuessAltText(value, /* isInitial = */ false); - }); - this.#overlayManager.register(dialog); } @@ -251,12 +247,13 @@ class NewAltTextManager { this.#imageData, /* updateAltTextData = */ false ); - if (altText) { - this.#guessedAltText = altText; - this.#wasAILoading = this.#isAILoading; - if (this.#isAILoading) { - this.#addAltText(altText); - } + if (altText === null) { + throw new Error("No valid response from the AI service."); + } + this.#guessedAltText = altText; + this.#wasAILoading = this.#isAILoading; + if (this.#isAILoading) { + this.#addAltText(altText); } } catch (e) { console.error(e); @@ -461,8 +458,6 @@ class ImageAltTextSettings { #createModelButton; - #downloadModelButton; - #dialog; #eventBus; @@ -491,7 +486,6 @@ class ImageAltTextSettings { this.#dialog = dialog; this.#aiModelSettings = aiModelSettings; this.#createModelButton = createModelButton; - this.#downloadModelButton = downloadModelButton; this.#showAltTextDialogButton = showAltTextDialogButton; this.#overlayManager = overlayManager; this.#eventBus = eventBus; @@ -514,61 +508,40 @@ class ImageAltTextSettings { this.#togglePref.bind(this, "enableNewAltTextWhenAddingImage") ); - deleteModelButton.addEventListener("click", this.#delete.bind(this, true)); - downloadModelButton.addEventListener( - "click", - this.#download.bind(this, true) - ); - - closeButton.addEventListener("click", this.#finish.bind(this)); + deleteModelButton.addEventListener("click", async () => { + await mlManager.deleteModel("altText"); - eventBus._on("enablealttextmodeldownload", ({ value }) => { - if (value) { - this.#download(false); - } else { - this.#delete(false); - } + aiModelSettings.classList.toggle("download", true); + createModelButton.disabled = true; + createModelButton.setAttribute("aria-pressed", false); + this.#setPref("enableGuessAltText", false); + this.#setPref("enableAltTextModelDownload", false); }); - this.#overlayManager.register(dialog); - } - - async #download(isFromUI = false) { - if (isFromUI) { - this.#downloadModelButton.disabled = true; - this.#downloadModelButton.firstChild.setAttribute( + downloadModelButton.addEventListener("click", async () => { + downloadModelButton.disabled = true; + downloadModelButton.firstChild.setAttribute( "data-l10n-id", "pdfjs-editor-alt-text-settings-downloading-model-button" ); - await this.#mlManager.downloadModel("altText"); + await mlManager.downloadModel("altText"); - this.#downloadModelButton.firstChild.setAttribute( + aiModelSettings.classList.toggle("download", false); + downloadModelButton.firstChild.setAttribute( "data-l10n-id", "pdfjs-editor-alt-text-settings-download-model-button" ); - - this.#createModelButton.disabled = false; + createModelButton.disabled = false; + createModelButton.setAttribute("aria-pressed", true); this.#setPref("enableGuessAltText", true); - this.#mlManager.toggleService("altText", true); + mlManager.toggleService("altText", true); this.#setPref("enableAltTextModelDownload", true); - this.#downloadModelButton.disabled = false; - } - - this.#aiModelSettings.classList.toggle("download", false); - this.#createModelButton.setAttribute("aria-pressed", true); - } - - async #delete(isFromUI = false) { - if (isFromUI) { - await this.#mlManager.deleteModel("altText"); - this.#setPref("enableGuessAltText", false); - this.#setPref("enableAltTextModelDownload", false); - } + downloadModelButton.disabled = false; + }); - this.#aiModelSettings.classList.toggle("download", true); - this.#createModelButton.disabled = true; - this.#createModelButton.setAttribute("aria-pressed", false); + closeButton.addEventListener("click", this.#finish.bind(this)); + this.#overlayManager.register(dialog); } async open({ enableGuessAltText, enableNewAltTextWhenAddingImage }) {