From d0bb3292922113d6617f3a11d305c46663e15e69 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Wed, 3 Jun 2026 09:07:29 -0400 Subject: [PATCH 01/11] Bug 2044471 - Add loading indicator to embedded dependency tree --- js/dependency-tree.js | 28 ++++++++++++++++++++++++---- skins/standard/dependency-tree.css | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index e6de7e58ef..89c2fd3eca 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -103,6 +103,11 @@ Bugzilla.DependencyTree = class DependencyTree { this.updateControllers({ maxDepth, hideResolved }); } + /** + * Error message to display when the dependency tree fails to load. + */ + #UPDATE_TREES_ERROR_MESSAGE = '

Failed to load the dependency tree.

'; + /** * Fetch and update the dependency tree HTML based on the given parameters, then inject it into * the page. @@ -119,11 +124,26 @@ Bugzilla.DependencyTree = class DependencyTree { }); const url = `${this.data.action}?${params}`; - const response = await fetch(`${url}&embed=1&tree_only=1`); - const html = response.ok ? await response.text() : undefined; - // Safe to inject HTML as is: same-origin fetch, Template Toolkit escapes all user-supplied data - this.$container.innerHTML = html ?? '

Failed to load the dependency tree.

'; + // Set up a delayed loading indicator — only show after 300ms to avoid flicker on fast loads + const loadingTimeout = setTimeout(() => { + this.$container.setAttribute('aria-busy', 'true'); + }, 300); + + try { + const response = await fetch(`${url}&embed=1&tree_only=1`); + const html = response.ok ? await response.text() : undefined; + + // Safe to inject HTML as is: same-origin fetch, Template Toolkit escapes all user-supplied data + this.$container.innerHTML = html ?? this.#UPDATE_TREES_ERROR_MESSAGE; + } catch { + this.$container.innerHTML = this.#UPDATE_TREES_ERROR_MESSAGE; + } finally { + // Cancel the loading timeout if it hasn’t fired yet + clearTimeout(loadingTimeout); + // Remove the loading state if it was set + this.$container.removeAttribute('aria-busy'); + } // Update the URL query parameters if we’re on the dependency tree page if (location.pathname === this.data.action) { diff --git a/skins/standard/dependency-tree.css b/skins/standard/dependency-tree.css index 65c78d8270..2adbfc2c0d 100644 --- a/skins/standard/dependency-tree.css +++ b/skins/standard/dependency-tree.css @@ -27,6 +27,28 @@ gap: 4px; } +#dependency-tree .tree-container { + position: relative; +} + +#dependency-tree .tree-container[aria-busy="true"]::before { + content: 'Updating…'; + position: absolute; + top: 50%; + left: 50%; + transform: translate(-50%, -50%); + border-radius: 4px; + padding: 8px 16px; + color: var(--menu-foreground-color); + background-color: var(--menu-background-color); + box-shadow: var(--menu-box-shadow); +} + +#dependency-tree .tree-container[aria-busy="true"] > [role="group"] { + opacity: 0.5; + pointer-events: none; +} + #dependency-tree .tree-container > [role="group"] { margin-block: 16px 0; } From 77d94563c94655c70ee199b21754529e80a5ce88 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Wed, 3 Jun 2026 09:14:45 -0400 Subject: [PATCH 02/11] [skip ci] Adjust loader vertical position --- skins/standard/dependency-tree.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skins/standard/dependency-tree.css b/skins/standard/dependency-tree.css index 2adbfc2c0d..6dfa6fe14e 100644 --- a/skins/standard/dependency-tree.css +++ b/skins/standard/dependency-tree.css @@ -34,7 +34,7 @@ #dependency-tree .tree-container[aria-busy="true"]::before { content: 'Updating…'; position: absolute; - top: 50%; + top: 100px; left: 50%; transform: translate(-50%, -50%); border-radius: 4px; From 4b13ee2a4f7e6d18977b346216e342f687a21158 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Wed, 3 Jun 2026 17:29:57 -0400 Subject: [PATCH 03/11] Center loading and error messages; disable controls while loading --- js/dependency-tree.js | 117 ++++++++++++++++++++++++++--- skins/standard/dependency-tree.css | 14 +++- 2 files changed, 119 insertions(+), 12 deletions(-) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index 89c2fd3eca..90d638ebb3 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -99,14 +99,93 @@ Bugzilla.DependencyTree = class DependencyTree { break; } + this.toggleControllers(true); await this.updateTrees({ maxDepth, hideResolved }); + this.toggleControllers(false); this.updateControllers({ maxDepth, hideResolved }); } /** - * Error message to display when the dependency tree fails to load. + * Get the visible height of the tree container using an `IntersectionObserver` so that we can + * position the loading indicator and error message in the center of the container. + * @returns {number} The height of the tree container in pixels. */ - #UPDATE_TREES_ERROR_MESSAGE = '

Failed to load the dependency tree.

'; + async getContainerHeight() { + let observer; + + return Promise.race([ + new Promise((resolve) => { + observer = new IntersectionObserver((entries) => { + entries.forEach((entry) => { + resolve(entry.intersectionRect.height); + observer.disconnect(); + }); + }); + + observer.observe(this.$container); + }), + new Promise((resolve) => { + // Fallback: use offsetHeight if observer doesn’t fire within 100ms, e.g. when the container + // is not visible + setTimeout(() => { + resolve(this.$container.clientHeight); + observer?.disconnect(); + }, 100); + }), + ]); + } + + /** + * Show an error message in the tree container if fetching the dependency tree fails. Position the + * message in the center of the container. + */ + async showErrorMessage() { + const containerHeight = await this.getContainerHeight(); + + this.$errorMessage ??= Object.assign(document.createElement('p'), { + className: 'error', + role: 'alert', + textContent: 'Failed to load the dependency tree.', + }); + + this.$errorMessage.style.top = `${containerHeight / 2}px`; + this.$container.innerHTML = ''; + this.$container.insertAdjacentElement('afterbegin', this.$errorMessage); + } + + /** + * Hide the error message if it is currently shown. + */ + hideErrorMessage() { + this.$errorMessage?.remove(); + } + + /** + * Show a loading message in the tree container while the dependency tree is being updated. + * Position the message in the center of the container. + */ + async showUpdatingMessage() { + const containerHeight = await this.getContainerHeight(); + + this.$updatingMessage ??= Object.assign(document.createElement('p'), { + className: 'updating', + role: 'status', + ariaLabel: 'Updating the dependency tree', + textContent: 'Updating…', + }); + + this.$updatingMessage.style.top = `${containerHeight / 2}px`; + this.$container.setAttribute('aria-busy', 'true'); + this.$container.insertAdjacentElement('afterbegin', this.$updatingMessage); + } + + /** + * Hide the loading message if it is currently shown and remove the busy state from the container. + */ + hideUpdatingMessage() { + this.$updatingMessage?.remove(); + this.$container.removeAttribute('aria-busy'); + } /** * Fetch and update the dependency tree HTML based on the given parameters, then inject it into @@ -125,24 +204,32 @@ Bugzilla.DependencyTree = class DependencyTree { const url = `${this.data.action}?${params}`; + // Hide any existing error message before starting a new fetch + this.hideErrorMessage(); + // Set up a delayed loading indicator — only show after 300ms to avoid flicker on fast loads const loadingTimeout = setTimeout(() => { - this.$container.setAttribute('aria-busy', 'true'); + this.showUpdatingMessage(); }, 300); try { const response = await fetch(`${url}&embed=1&tree_only=1`); - const html = response.ok ? await response.text() : undefined; - // Safe to inject HTML as is: same-origin fetch, Template Toolkit escapes all user-supplied data - this.$container.innerHTML = html ?? this.#UPDATE_TREES_ERROR_MESSAGE; - } catch { - this.$container.innerHTML = this.#UPDATE_TREES_ERROR_MESSAGE; + if (response.ok) { + // Safe to inject HTML as is: Template Toolkit escapes all user-supplied data + this.$container.innerHTML = await response.text(); + } else { + console.error('Failed to fetch dependency tree:', response.status); + this.showErrorMessage(); + } + } catch (ex) { + console.error('Error fetching dependency tree:', ex); + this.showErrorMessage(); } finally { // Cancel the loading timeout if it hasn’t fired yet clearTimeout(loadingTimeout); // Remove the loading state if it was set - this.$container.removeAttribute('aria-busy'); + this.hideUpdatingMessage(); } // Update the URL query parameters if we’re on the dependency tree page @@ -168,6 +255,18 @@ Bugzilla.DependencyTree = class DependencyTree { this.$removeLimitBtn.disabled = maxDepth === 0 || maxDepth === this.realDepth; } + /** + * Enable or disable the toolbar buttons and inputs. This is used to prevent multiple simultaneous + * updates while one is already in progress. + * @param {boolean} disabled Whether to disable the controllers. + */ + toggleControllers(disabled) { + this.$toggleBtn.disabled = disabled; + this.$setLimitBtn.disabled = disabled || this.realDepth < 2 || this.data.maxDepth === 1; + this.$removeLimitBtn.disabled = disabled || this.data.maxDepth === 0 || this.data.maxDepth === this.realDepth; + this.$numberInput.disabled = disabled; + } + /** * Attach event listeners to the tree items to handle expanding/collapsing and highlighting * duplicates. Use event delegation to handle events on dynamically updated tree items. diff --git a/skins/standard/dependency-tree.css b/skins/standard/dependency-tree.css index 6dfa6fe14e..beba3f77e7 100644 --- a/skins/standard/dependency-tree.css +++ b/skins/standard/dependency-tree.css @@ -29,14 +29,18 @@ #dependency-tree .tree-container { position: relative; + min-height: 100px; /* Ensure the container has some height for the loading indicator and error message to be positioned */ } -#dependency-tree .tree-container[aria-busy="true"]::before { - content: 'Updating…'; +#dependency-tree .tree-container .updating, +#dependency-tree .tree-container .error { position: absolute; - top: 100px; + top: 0; /* To be updated dynamically in JS */ left: 50%; transform: translate(-50%, -50%); +} + +#dependency-tree .tree-container .updating { border-radius: 4px; padding: 8px 16px; color: var(--menu-foreground-color); @@ -44,6 +48,10 @@ box-shadow: var(--menu-box-shadow); } +#dependency-tree .tree-container .error { + color: var(--error-message-foreground-color); +} + #dependency-tree .tree-container[aria-busy="true"] > [role="group"] { opacity: 0.5; pointer-events: none; From a3864e02a8d25ad9391580d2cd0c599e1080e1b0 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Wed, 3 Jun 2026 17:39:15 -0400 Subject: [PATCH 04/11] Refactor controller toggling and disabling --- js/dependency-tree.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index 90d638ebb3..505e1ff32e 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -99,9 +99,8 @@ Bugzilla.DependencyTree = class DependencyTree { break; } - this.toggleControllers(true); + this.disableControllers(); await this.updateTrees({ maxDepth, hideResolved }); - this.toggleControllers(false); this.updateControllers({ maxDepth, hideResolved }); } @@ -238,6 +237,16 @@ Bugzilla.DependencyTree = class DependencyTree { } } + /** + * Temporarily disable all toolbar buttons and inputs to prevent multiple simultaneous updates. + */ + disableControllers() { + this.$toggleBtn.disabled = true; + this.$setLimitBtn.disabled = true; + this.$removeLimitBtn.disabled = true; + this.$numberInput.disabled = true; + } + /** * Update the state of the toolbar buttons and inputs based on the current parameters. * @param {object} params Parameters. @@ -251,20 +260,10 @@ Bugzilla.DependencyTree = class DependencyTree { // Update button states this.$toggleBtn.textContent = hideResolved ? 'Show Resolved' : 'Hide Resolved'; + this.$toggleBtn.disabled = false; this.$setLimitBtn.disabled = this.realDepth < 2 || maxDepth === 1; this.$removeLimitBtn.disabled = maxDepth === 0 || maxDepth === this.realDepth; - } - - /** - * Enable or disable the toolbar buttons and inputs. This is used to prevent multiple simultaneous - * updates while one is already in progress. - * @param {boolean} disabled Whether to disable the controllers. - */ - toggleControllers(disabled) { - this.$toggleBtn.disabled = disabled; - this.$setLimitBtn.disabled = disabled || this.realDepth < 2 || this.data.maxDepth === 1; - this.$removeLimitBtn.disabled = disabled || this.data.maxDepth === 0 || this.data.maxDepth === this.realDepth; - this.$numberInput.disabled = disabled; + this.$numberInput.disabled = false; } /** From 95270bee3832fdeff2df326a3a545ca596419d77 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Wed, 3 Jun 2026 17:49:05 -0400 Subject: [PATCH 05/11] [skip ci] Add z-index to dependency tree marker --- skins/standard/dependency-tree.css | 1 + 1 file changed, 1 insertion(+) diff --git a/skins/standard/dependency-tree.css b/skins/standard/dependency-tree.css index beba3f77e7..da6f153309 100644 --- a/skins/standard/dependency-tree.css +++ b/skins/standard/dependency-tree.css @@ -37,6 +37,7 @@ position: absolute; top: 0; /* To be updated dynamically in JS */ left: 50%; + z-index: 10; transform: translate(-50%, -50%); } From cf1a0c9ad87948570033548afff59ddd192f46d6 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Wed, 3 Jun 2026 17:55:45 -0400 Subject: [PATCH 06/11] [skip ci] Limit dependency tree container height --- extensions/BugModal/web/bug_modal.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extensions/BugModal/web/bug_modal.css b/extensions/BugModal/web/bug_modal.css index 151642b2fa..1010d2e045 100644 --- a/extensions/BugModal/web/bug_modal.css +++ b/extensions/BugModal/web/bug_modal.css @@ -569,6 +569,11 @@ input[type="number"] { font-size: inherit; } +#dependency-tree-container .tree-container { + max-height: 80dvh; /* Limit the height of the tree container to 80% of the viewport height */ + overflow: auto; +} + #hide-dependency-tree-btn { margin-left: auto; } From 81017687591b3ed8e8b6318d7c31e7303c7eda9b Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 8 Jun 2026 10:01:25 -0400 Subject: [PATCH 07/11] Fix race condition and unawaited promise --- js/dependency-tree.js | 106 +++++++++++++++++++++++++++++++----------- 1 file changed, 78 insertions(+), 28 deletions(-) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index 505e1ff32e..dfd374ece5 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -27,7 +27,6 @@ Bugzilla.DependencyTree = class DependencyTree { return; } - this.data = this.$trees.dataset; this.data.initialized = '1'; this.realDepth = Number(this.data.realDepth); @@ -38,6 +37,9 @@ Bugzilla.DependencyTree = class DependencyTree { this.activateToolbar(); this.activateTrees(); + + // Track the current update request to prevent race conditions with `showUpdatingMessage()` + this.updateGeneration = 0; } /** @@ -135,21 +137,65 @@ Bugzilla.DependencyTree = class DependencyTree { } /** - * Show an error message in the tree container if fetching the dependency tree fails. Position the - * message in the center of the container. + * Show or update a message in the tree container with generation-based lifecycle management. + * Positions the message in the center of the container. + * @param {object} config Configuration object. + * @param {number} config.generation The generation ID of the current update request. + * @param {string} config.messageType The type of message ('error' or 'updating'). + * @param {object} config.element Element properties (`className`, `role`, `textContent`, etc.). + * @param {boolean} [config.clearContainer] Whether to clear container `innerHTML` before + * inserting. + * @param {() => void} [config.onShow] Optional callback to run after the message is shown. */ - async showErrorMessage() { + async showMessage({ + generation, + messageType, + element, + clearContainer = false, + onShow = undefined, + }) { + // Don’t proceed if a newer request has already started + if (generation !== this.updateGeneration) { + return; + } + const containerHeight = await this.getContainerHeight(); - this.$errorMessage ??= Object.assign(document.createElement('p'), { - className: 'error', - role: 'alert', - textContent: 'Failed to load the dependency tree.', - }); + // Check again after async operation to ensure this request is still current + if (generation !== this.updateGeneration) { + return; + } + + const fieldName = `$${messageType}Message`; + + this[fieldName] ??= Object.assign(document.createElement('p'), element); + this[fieldName].style.top = `${containerHeight / 2}px`; + + if (clearContainer) { + this.$container.innerHTML = ''; + } + + // Insert the message + this.$container.insertAdjacentElement('afterbegin', this[fieldName]); - this.$errorMessage.style.top = `${containerHeight / 2}px`; - this.$container.innerHTML = ''; - this.$container.insertAdjacentElement('afterbegin', this.$errorMessage); + onShow?.(); + } + + /** + * Show an error message in the tree container if fetching the dependency tree fails. + * @param {number} generation The generation ID of the current update request. + */ + async showErrorMessage(generation) { + await this.showMessage({ + generation, + messageType: 'error', + element: { + className: 'error', + role: 'alert', + textContent: 'Failed to load the dependency tree.', + }, + clearContainer: true, + }); } /** @@ -161,21 +207,22 @@ Bugzilla.DependencyTree = class DependencyTree { /** * Show a loading message in the tree container while the dependency tree is being updated. - * Position the message in the center of the container. + * @param {number} generation The generation ID of the current update request. */ - async showUpdatingMessage() { - const containerHeight = await this.getContainerHeight(); - - this.$updatingMessage ??= Object.assign(document.createElement('p'), { - className: 'updating', - role: 'status', - ariaLabel: 'Updating the dependency tree', - textContent: 'Updating…', + async showUpdatingMessage(generation) { + await this.showMessage({ + generation, + messageType: 'updating', + element: { + className: 'updating', + role: 'status', + ariaLabel: 'Updating the dependency tree', + textContent: 'Updating…', + }, + onShow: () => { + this.$container.setAttribute('aria-busy', 'true'); + }, }); - - this.$updatingMessage.style.top = `${containerHeight / 2}px`; - this.$container.setAttribute('aria-busy', 'true'); - this.$container.insertAdjacentElement('afterbegin', this.$updatingMessage); } /** @@ -203,12 +250,15 @@ Bugzilla.DependencyTree = class DependencyTree { const url = `${this.data.action}?${params}`; + // Increment generation counter to invalidate any in-flight message operations + const generation = ++this.updateGeneration; + // Hide any existing error message before starting a new fetch this.hideErrorMessage(); // Set up a delayed loading indicator — only show after 300ms to avoid flicker on fast loads const loadingTimeout = setTimeout(() => { - this.showUpdatingMessage(); + this.showUpdatingMessage(generation); }, 300); try { @@ -219,11 +269,11 @@ Bugzilla.DependencyTree = class DependencyTree { this.$container.innerHTML = await response.text(); } else { console.error('Failed to fetch dependency tree:', response.status); - this.showErrorMessage(); + await this.showErrorMessage(generation); } } catch (ex) { console.error('Error fetching dependency tree:', ex); - this.showErrorMessage(); + await this.showErrorMessage(generation); } finally { // Cancel the loading timeout if it hasn’t fired yet clearTimeout(loadingTimeout); From 59ec27d0d6b0e8fb83ca2a57cf54919f0601a4a1 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 8 Jun 2026 10:14:25 -0400 Subject: [PATCH 08/11] Remove async/await from event handlers --- js/dependency-tree.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index dfd374ece5..68ca53283c 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -51,14 +51,14 @@ Bugzilla.DependencyTree = class DependencyTree { this.$removeLimitBtn = this.$toolbar.querySelector('[data-id="remove-limit"]'); this.$numberInput = this.$toolbar.querySelector('[data-id="custom-limit"]'); - this.$toolbar.addEventListener('click', async ({ target }) => { + this.$toolbar.addEventListener('click', ({ target }) => { if (target.matches('button[type="button"]')) { - await this.onAction(target.dataset.id); + this.onAction(target.dataset.id); } }); - this.$numberInput?.addEventListener('change', async () => { - await this.onAction('change-limit'); + this.$numberInput?.addEventListener('change', () => { + this.onAction('change-limit'); }); } From 20a1c3e55d675d02c72a0ad7644c87d17ab243f7 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 8 Jun 2026 10:14:52 -0400 Subject: [PATCH 09/11] Prevent Enter from submitting limit input --- js/dependency-tree.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index 68ca53283c..e6b35e316a 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -60,6 +60,14 @@ Bugzilla.DependencyTree = class DependencyTree { this.$numberInput?.addEventListener('change', () => { this.onAction('change-limit'); }); + + this.$numberInput?.addEventListener('keydown', (event) => { + // Prevent form submission on Enter and trigger the limit change action instead + if (event.key === 'Enter') { + event.preventDefault(); + this.onAction('change-limit'); + } + }); } /** From 5e213933bef10dd449668feedb305e8e5ebf7a8a Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 8 Jun 2026 10:17:18 -0400 Subject: [PATCH 10/11] Validate depth input in DependencyTree --- js/dependency-tree.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index e6b35e316a..cf1446da4b 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -102,6 +102,13 @@ Bugzilla.DependencyTree = class DependencyTree { case 'change-limit': maxDepth = Number(this.$numberInput?.value || this.realDepth); + // Validate that the value is within the acceptable range + if (maxDepth < 1 || maxDepth > this.realDepth) { + // Reset to the current valid value and bail out + this.$numberInput.value = this.data.maxDepth > 0 ? this.data.maxDepth : this.realDepth; + return; + } + if (maxDepth === this.realDepth) { removeLimit(); } From fccf335f0780aa7346be8de4ef2b20777606305f Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 8 Jun 2026 16:43:22 -0400 Subject: [PATCH 11/11] Bump updateGeneration on request completion --- js/dependency-tree.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/dependency-tree.js b/js/dependency-tree.js index cf1446da4b..acb9b66321 100644 --- a/js/dependency-tree.js +++ b/js/dependency-tree.js @@ -290,6 +290,8 @@ Bugzilla.DependencyTree = class DependencyTree { console.error('Error fetching dependency tree:', ex); await this.showErrorMessage(generation); } finally { + // Increment generation to invalidate any in-flight message operations from this request + this.updateGeneration++; // Cancel the loading timeout if it hasn’t fired yet clearTimeout(loadingTimeout); // Remove the loading state if it was set