From 65342d2beeff916705086f576099485d3e27a886 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 27 Feb 2024 21:44:13 +0100 Subject: [PATCH] [Editor] Add some telemetry for the highlight feature (bug 1866437) --- src/display/annotation_storage.js | 35 ++++++++++++ src/display/editor/alt_text.js | 11 +--- src/display/editor/annotation_editor_layer.js | 1 + src/display/editor/editor.js | 57 +++++++++++++++++++ src/display/editor/highlight.js | 46 +++++++++++++++ src/display/editor/stamp.js | 11 +--- src/display/editor/tools.js | 42 +++++++++----- web/alt_text_manager.js | 17 ++---- web/app.js | 17 +++--- 9 files changed, 186 insertions(+), 51 deletions(-) diff --git a/src/display/annotation_storage.js b/src/display/annotation_storage.js index e2a80b230..13032afab 100644 --- a/src/display/annotation_storage.js +++ b/src/display/annotation_storage.js @@ -212,6 +212,41 @@ class AnnotationStorage { ? { map, hash: hash.hexdigest(), transfer } : SerializableEmpty; } + + get editorStats() { + const stats = Object.create(null); + const typeToEditor = new Map(); + for (const value of this.#storage.values()) { + if (!(value instanceof AnnotationEditor)) { + continue; + } + const editorStats = value.telemetryFinalData; + if (!editorStats) { + continue; + } + const { type } = editorStats; + if (!typeToEditor.has(type)) { + typeToEditor.set(type, Object.getPrototypeOf(value).constructor); + } + const map = (stats[type] ||= new Map()); + for (const [key, val] of Object.entries(editorStats)) { + if (key === "type") { + continue; + } + let counters = map.get(key); + if (!counters) { + counters = new Map(); + map.set(key, counters); + } + const count = counters.get(val) ?? 0; + counters.set(val, count + 1); + } + } + for (const [type, editor] of typeToEditor) { + stats[type] = editor.computeTelemetryFinalData(stats[type]); + } + return stats; + } } /** diff --git a/src/display/editor/alt_text.js b/src/display/editor/alt_text.js index c720b93d0..428f2f20d 100644 --- a/src/display/editor/alt_text.js +++ b/src/display/editor/alt_text.js @@ -146,15 +146,8 @@ class AltText { this.#altTextTooltipTimeout = setTimeout(() => { this.#altTextTooltipTimeout = null; this.#altTextTooltip.classList.add("show"); - this.#editor._uiManager._eventBus.dispatch("reporttelemetry", { - source: this, - details: { - type: "editing", - subtype: this.#editor.editorType, - data: { - action: "alt_text_tooltip", - }, - }, + this.#editor._reportTelemetry({ + action: "alt_text_tooltip", }); }, DELAY_TO_SHOW_TOOLTIP); }); diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 489c781c7..b7f7d4043 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -471,6 +471,7 @@ class AnnotationEditorLayer { editor.fixAndSetPosition(); editor.onceAdded(); this.#uiManager.addToAnnotationStorage(editor); + editor._reportTelemetry(editor.telemetryInitialData); } moveEditorInDOM(editor) { diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 5a20f8764..0c8e61cd4 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -72,6 +72,8 @@ class AnnotationEditor { #prevDragY = 0; + #telemetryTimeouts = null; + _initialOptions = Object.create(null); _uiManager = null; @@ -90,6 +92,11 @@ class AnnotationEditor { static _zIndex = 1; + // Time to wait (in ms) before sending the telemetry data. + // We wait a bit to avoid sending too many requests when changing something + // like the thickness of a line. + static _telemetryTimeout = 1000; + static get _resizerKeyboardManager() { const resize = AnnotationEditor.prototype._resizeWithKeyboard; const small = AnnotationEditorUIManager.TRANSLATE_SMALL; @@ -1323,6 +1330,12 @@ class AnnotationEditor { } this.#stopResizing(); this.removeEditToolbar(); + if (this.#telemetryTimeouts) { + for (const timeout of this.#telemetryTimeouts.values()) { + clearTimeout(timeout); + } + this.#telemetryTimeouts = null; + } } /** @@ -1598,6 +1611,50 @@ class AnnotationEditor { static canCreateNewEmptyEditor() { return true; } + + /** + * Get the data to report to the telemetry when the editor is added. + * @returns {Object} + */ + get telemetryInitialData() { + return { action: "added" }; + } + + /** + * The telemetry data to use when saving/printing. + * @returns {Object|null} + */ + get telemetryFinalData() { + return null; + } + + _reportTelemetry(data, mustWait = false) { + if (mustWait) { + this.#telemetryTimeouts ||= new Map(); + const { action } = data; + let timeout = this.#telemetryTimeouts.get(action); + if (timeout) { + clearTimeout(timeout); + } + timeout = setTimeout(() => { + this._reportTelemetry(data); + this.#telemetryTimeouts.delete(action); + if (this.#telemetryTimeouts.size === 0) { + this.#telemetryTimeouts = null; + } + }, AnnotationEditor._telemetryTimeout); + this.#telemetryTimeouts.set(action, timeout); + return; + } + data.type ||= this.editorType; + this._uiManager._eventBus.dispatch("reporttelemetry", { + source: this, + details: { + type: "editing", + data, + }, + }); + } } // This class is used to fake an editor which has been deleted. diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js index 349d09a12..f0a76e616 100644 --- a/src/display/editor/highlight.js +++ b/src/display/editor/highlight.js @@ -52,6 +52,8 @@ class HighlightEditor extends AnnotationEditor { #thickness; + #methodOfCreation = ""; + static _defaultColor = null; static _defaultOpacity = 1; @@ -76,6 +78,7 @@ class HighlightEditor extends AnnotationEditor { this.#thickness = params.thickness || HighlightEditor._defaultThickness; this.#opacity = params.opacity || HighlightEditor._defaultOpacity; this.#boxes = params.boxes || null; + this.#methodOfCreation = params.methodOfCreation || ""; this._isDraggable = false; if (params.highlightId > -1) { @@ -89,6 +92,34 @@ class HighlightEditor extends AnnotationEditor { } } + /** @inheritdoc */ + get telemetryInitialData() { + return { + action: "added", + type: this.#telemetryType, + color: this._uiManager.highlightColorNames.get(this.color), + thickness: this.#thickness, + methodOfCreation: this.#methodOfCreation, + }; + } + + /** @inheritdoc */ + get telemetryFinalData() { + return { + type: this.#telemetryType, + color: this._uiManager.highlightColorNames.get(this.color), + }; + } + + static computeTelemetryFinalData(data) { + // We want to know how many colors have been used. + return { numberOfColors: data.get("color").size }; + } + + get #telemetryType() { + return this.#isFreeHighlight ? "free_highlight" : "highlight"; + } + #createOutlines() { const outliner = new Outliner(this.#boxes, /* borderWidth = */ 0.001); this.#highlightOutlines = outliner.getOutlines(); @@ -274,6 +305,14 @@ class HighlightEditor extends AnnotationEditor { overwriteIfSameType: true, keepUndo: true, }); + + this._reportTelemetry( + { + action: "color_changed", + color: this._uiManager.highlightColorNames.get(color), + }, + /* mustWait = */ true + ); } /** @@ -295,6 +334,10 @@ class HighlightEditor extends AnnotationEditor { overwriteIfSameType: true, keepUndo: true, }); + this._reportTelemetry( + { action: "thickness_changed", thickness }, + /* mustWait = */ true + ); } /** @inheritdoc */ @@ -349,6 +392,9 @@ class HighlightEditor extends AnnotationEditor { remove() { super.remove(); this.#cleanDrawLayer(); + this._reportTelemetry({ + action: "deleted", + }); } /** @inheritdoc */ diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index 132610f85..464bc58cb 100644 --- a/src/display/editor/stamp.js +++ b/src/display/editor/stamp.js @@ -326,15 +326,8 @@ class StampEditor extends AnnotationEditor { // There are multiple ways to add an image to the page, so here we just // count the number of times an image is added to the page whatever the way // is. - this._uiManager._eventBus.dispatch("reporttelemetry", { - source: this, - details: { - type: "editing", - subtype: this.editorType, - data: { - action: "inserted_image", - }, - }, + this._reportTelemetry({ + action: "inserted_image", }); this.addAltTextButton(); if (this.#bitmapFileName) { diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index 2fb48452e..0040d53f5 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -871,6 +871,16 @@ class AnnotationEditorUIManager { ); } + get highlightColorNames() { + return shadow( + this, + "highlightColorNames", + this.highlightColors + ? new Map(Array.from(this.highlightColors, e => e.reverse())) + : null + ); + } + setMainHighlightColorPicker(colorPicker) { this.#mainHighlightColorPicker = colorPicker; } @@ -932,7 +942,7 @@ class AnnotationEditorUIManager { this.viewParameters.rotation = pagesRotation; } - highlightSelection() { + highlightSelection(methodOfCreation = "") { const selection = document.getSelection(); if (!selection || selection.isCollapsed) { return; @@ -953,7 +963,10 @@ class AnnotationEditorUIManager { } for (const layer of this.#allLayers.values()) { if (layer.hasTextLayer(textLayer)) { - layer.createAndAddNewEditor({ x: 0, y: 0 }, false, { boxes }); + layer.createAndAddNewEditor({ x: 0, y: 0 }, false, { + methodOfCreation, + boxes, + }); break; } } @@ -1020,7 +1033,7 @@ class AnnotationEditorUIManager { window.removeEventListener("pointerup", pointerup); window.removeEventListener("blur", pointerup); if (e.type === "pointerup") { - this.highlightSelection(); + this.highlightSelection("main_toolbar"); } }; window.addEventListener("pointerup", pointerup); @@ -1050,7 +1063,7 @@ class AnnotationEditorUIManager { this.isShiftKeyDown = false; if (this.#highlightWhenShiftUp) { this.#highlightWhenShiftUp = false; - this.highlightSelection(); + this.highlightSelection("main_toolbar"); } if (!this.hasSelection) { return; @@ -1240,7 +1253,7 @@ class AnnotationEditorUIManager { this.isShiftKeyDown = false; if (this.#highlightWhenShiftUp) { this.#highlightWhenShiftUp = false; - this.highlightSelection(); + this.highlightSelection("main_toolbar"); } } } @@ -1249,15 +1262,18 @@ class AnnotationEditorUIManager { * Execute an action for a given name. * For example, the user can click on the "Undo" entry in the context menu * and it'll trigger the undo action. - * @param {Object} details */ - onEditingAction(details) { - if ( - ["undo", "redo", "delete", "selectAll", "highlightSelection"].includes( - details.name - ) - ) { - this[details.name](); + onEditingAction({ name }) { + switch (name) { + case "undo": + case "redo": + case "delete": + case "selectAll": + this[name](); + break; + case "highlightSelection": + this.highlightSelection("context_menu"); + break; } } diff --git a/web/alt_text_manager.js b/web/alt_text_manager.js index a8e165268..b487fadd0 100644 --- a/web/alt_text_manager.js +++ b/web/alt_text_manager.js @@ -248,17 +248,12 @@ class AltTextManager { } #close() { - this.#eventBus.dispatch("reporttelemetry", { - source: this, - details: { - type: "editing", - subtype: this.#currentEditor.editorType, - data: this.#telemetryData || { - action: "alt_text_cancel", - alt_text_keyboard: !this.#hasUsedPointer, - }, - }, - }); + this.#currentEditor._reportTelemetry( + this.#telemetryData || { + action: "alt_text_cancel", + alt_text_keyboard: !this.#hasUsedPointer, + } + ); this.#telemetryData = null; this.#removeOnClickListeners(); diff --git a/web/app.js b/web/app.js index 1cd3447d9..05a080ee6 100644 --- a/web/app.js +++ b/web/app.js @@ -1152,7 +1152,10 @@ const PDFViewerApplication = { if (this._hasAnnotationEditors) { this.externalServices.reportTelemetry({ type: "editing", - data: { type: "save" }, + data: { + type: "save", + stats: this.pdfDocument?.annotationStorage.editorStats, + }, }); } }, @@ -1727,13 +1730,6 @@ const PDFViewerApplication = { annotationStorage.onAnnotationEditor = typeStr => { this._hasAnnotationEditors = !!typeStr; this.setTitle(); - - if (typeStr) { - this.externalServices.reportTelemetry({ - type: "editing", - data: { type: typeStr }, - }); - } }; }, @@ -1857,7 +1853,10 @@ const PDFViewerApplication = { if (this._hasAnnotationEditors) { this.externalServices.reportTelemetry({ type: "editing", - data: { type: "print" }, + data: { + type: "print", + stats: this.pdfDocument?.annotationStorage.editorStats, + }, }); } },