From e161826a44d9d902c2b479c4da949d8774b56886 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 3 Dec 2024 16:11:41 +0100 Subject: [PATCH] [Editor] Corrrectly get the words from the alt-text when reporting the telemetry (bug 1929311) --- test/integration/stamp_editor_spec.mjs | 82 +++++++++++++++++++++++++- web/genericcom.js | 2 +- web/new_alt_text_manager.js | 13 +++- 3 files changed, 93 insertions(+), 4 deletions(-) diff --git a/test/integration/stamp_editor_spec.mjs b/test/integration/stamp_editor_spec.mjs index d09d9c3c37a0a..674366120efad 100644 --- a/test/integration/stamp_editor_spec.mjs +++ b/test/integration/stamp_editor_spec.mjs @@ -897,6 +897,9 @@ describe("Stamp Editor", () => { eventBus.on("annotationeditoruimanager", ({ uiManager }) => { window.uiManager = uiManager; }); + eventBus.on("reporttelemetry", ({ details }) => { + (window.telemetry ||= []).push(structuredClone(details)); + }); }, }, { @@ -917,6 +920,7 @@ describe("Stamp Editor", () => { } await page.evaluate(() => { window.uiManager.reset(); + window.telemetry = []; }); // Disable editing mode. await switchToStamp(page, /* disable */ true); @@ -953,7 +957,7 @@ describe("Stamp Editor", () => { // Check that AI guessed the correct alt text. await page.waitForFunction( `document.getElementById("newAltTextDescriptionTextarea").value === - "Fake alt text"` + "Fake alt text."` ); // Check that the dialog has the correct title: "Edit..." @@ -1182,6 +1186,82 @@ describe("Stamp Editor", () => { await page.waitForSelector("#newAltTextDisclaimer[hidden]"); } }); + + it("must check that the data in telemetry are correct", async () => { + // Run sequentially to avoid clipboard issues. + for (const [browserName, page] of pages) { + await page.evaluate(() => { + window.PDFViewerApplication.mlManager.enableAltTextModelDownload = true; + }); + await switchToStamp(page); + + // Add an image. + await copyImage(page, "../images/firefox_logo.png", 0); + const editorSelector = getEditorSelector(0); + await page.waitForSelector(editorSelector); + await waitForSerialized(page, 1); + + // Wait for the dialog to be visible. + await page.waitForSelector("#newAltTextDialog", { visible: true }); + + // Check that AI guessed the correct alt text. + await page.waitForFunction( + `document.getElementById("newAltTextDescriptionTextarea").value === + "Fake alt text."` + ); + // Clear the input and check that the title changes to "Add..." + await clearInput( + page, + "#newAltTextDescriptionTextarea", + /* waitForInputEvent = */ true + ); + // Save the empty text. + await page.click("#newAltTextSave"); + await page.waitForSelector("#newAltTextDialog", { visible: false }); + + // Get the telemetry data and clean. + let telemetry = await page.evaluate(() => { + const tel = window.telemetry; + window.telemetry = []; + return tel; + }); + let saveTelemetry = telemetry.find( + details => details.data.action === "pdfjs.image.alt_text.user_edit" + ); + expect(saveTelemetry.data.data) + .withContext(`In ${browserName}`) + .toEqual({ + total_words: 3, + words_removed: 3, + words_added: 0, + }); + + // Click on the Review button. + const buttonSelector = `${editorSelector} button.altText.new`; + await page.waitForSelector(buttonSelector, { visible: true }); + await page.click(buttonSelector); + await page.waitForSelector("#newAltTextDialog", { visible: true }); + + // Add a new alt text and check that the title changes to "Edit..." + await page.type("#newAltTextDescriptionTextarea", "Fake text alt foo."); + + // Save the empty text. + await page.click("#newAltTextSave"); + await page.waitForSelector("#newAltTextDialog", { visible: false }); + + telemetry = await page.evaluate(() => window.telemetry); + saveTelemetry = telemetry.find( + details => details.data.action === "pdfjs.image.alt_text.user_edit" + ); + expect(saveTelemetry.data.data) + .withContext(`In ${browserName}`) + .toEqual({ + total_words: 3, + words_removed: 0, + words_added: 1, + }); + } + }); }); describe("New alt-text flow (bug 1920515)", () => { diff --git a/web/genericcom.js b/web/genericcom.js index 9bddcbc456eb5..610dac9502140 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -132,7 +132,7 @@ class FakeMLManager { guess({ request: { data } }) { return new Promise(resolve => { setTimeout(() => { - resolve(data ? { output: "Fake alt text" } : { error: true }); + resolve(data ? { output: "Fake alt text." } : { error: true }); }, 3000); }); } diff --git a/web/new_alt_text_manager.js b/web/new_alt_text_manager.js index d9b7c80be406e..12d1eeaa51007 100644 --- a/web/new_alt_text_manager.js +++ b/web/new_alt_text_manager.js @@ -460,6 +460,15 @@ class NewAltTextManager { this.#uiManager = null; } + #extractWords(text) { + return new Set( + text + .toLowerCase() + .split(/[^\p{L}\p{N}]+/gu) + .filter(x => !!x) + ); + } + #save() { const altText = this.#textarea.value.trim(); this.#currentEditor.altTextData = { @@ -469,8 +478,8 @@ class NewAltTextManager { this.#currentEditor.altTextData.guessedAltText = this.#guessedAltText; if (this.#guessedAltText && this.#guessedAltText !== altText) { - const guessedWords = new Set(this.#guessedAltText.split(/\s+/)); - const words = new Set(altText.split(/\s+/)); + const guessedWords = this.#extractWords(this.#guessedAltText); + const words = this.#extractWords(altText); this.#currentEditor._reportTelemetry({ action: "pdfjs.image.alt_text.user_edit", data: {