commit 9b843587aa4623afff088bd86c5eddc7b8c5b728
parent 521c54a4ddafb5e33b7f6ffb39e3a87def963d15
Author: Tim Giles <tgiles@mozilla.com>
Date: Thu, 23 Oct 2025 18:42:41 +0000
Bug 1983388 - Prevent loss of focus when navigating to search pane. r=home-newtab-reviewers,akulyk,maxx,mconley
By changing the vbox id of the "Web Search" section, we prevent focus
from being stolen. I'm not entirely sure why this change fixes the issue
as it's likely something to do with the "7.4.6.4 Scrolling to a
fragment" part of the HTML spec. If I had to guess...it's because we end
up finding a valid focus target, due to the id of the vbox being "search".
Then we end up following the focusing steps and move the focus to the
document body.
I would bet there are no elements with ids that are the same as the hash
values we use to move between categories in about:preferences, thus
focus doesn't move because there are no valid focus targets in the
document.
See Comment #8 by :smaug for more detail.
Differential Revision: https://phabricator.services.mozilla.com/D265316
Diffstat:
5 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/browser/components/preferences/home.inc.xhtml b/browser/components/preferences/home.inc.xhtml
@@ -96,7 +96,7 @@
<label><html:h2 data-l10n-id="home-prefs-content-header2" /></label>
<description class="description-deemphasized" data-l10n-id="home-prefs-content-description2" />
- <vbox id="search" />
+ <vbox id="web-search" />
<vbox id="weather" />
<vbox id="trending-searches" />
<vbox id="topsites" />
diff --git a/browser/components/preferences/preferences.js b/browser/components/preferences/preferences.js
@@ -380,6 +380,10 @@ async function gotoPref(
// Overwrite the hash, unless there is no hash and we're switching to the
// default category, e.g. by using the 'back' button after navigating to
// a different category.
+
+ // Note: Bug 1983388 - If there is an element in the DOM that has the same
+ // ID as the `friendlyName`, then focus will be lost when navigating the
+ // category navigation via keyboard when that `friendlyName` category is selected.
if (
!(!document.location.hash && category == kDefaultCategoryInternalName)
) {
diff --git a/browser/components/preferences/tests/browser.toml b/browser/components/preferences/tests/browser.toml
@@ -150,6 +150,8 @@ fail-if = ["a11y_checks"] # Bug 1854636 clicked treechildren#engineChildren may
["browser_newtab_menu.js"]
+["browser_no_id_collision_between_categories_and_mainPane.js"]
+
["browser_notifications_do_not_disturb.js"]
["browser_open_download_preferences.js"]
diff --git a/browser/components/preferences/tests/browser_no_id_collision_between_categories_and_mainPane.js b/browser/components/preferences/tests/browser_no_id_collision_between_categories_and_mainPane.js
@@ -0,0 +1,31 @@
+/* Any copyright is dedicated to the Public Domain.
+ https://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+/*
+ * Bug 1983388 - Ensure focus isn't lost when navigating categories via keyboard.
+ * By checking that there are no elements in the document with an ID that is the
+ * same value as the category values, focus will not move from the richlistbox.
+ * */
+add_task(async function test_no_category_values_as_markup_ids() {
+ await openPreferencesViaOpenPreferencesAPI("paneGeneral", {
+ leaveOpen: true,
+ });
+
+ let categories = gBrowser.contentDocument.getElementById("categories");
+
+ let categoryValues = categories.itemChildren.map(elem => {
+ return gBrowser.contentWindow.internalPrefCategoryNameToFriendlyName(
+ elem.value
+ );
+ });
+
+ for (let value of categoryValues) {
+ ok(
+ !gBrowser.contentDocument.getElementById(value),
+ `There should be no markup with id: ${value}`
+ );
+ }
+
+ BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
diff --git a/browser/extensions/newtab/lib/AboutPreferences.sys.mjs b/browser/extensions/newtab/lib/AboutPreferences.sys.mjs
@@ -13,7 +13,7 @@ export const PREFERENCES_LOADED_EVENT = "home-pane-loaded";
// SectionsManager to construct the preferences view.
const PREFS_FOR_SETTINGS = () => [
{
- id: "search",
+ id: "web-search",
pref: {
feed: "showSearch",
titleString: "home-prefs-search-header",
@@ -224,9 +224,25 @@ export class AboutPreferences {
* @param document
* @param Preferences
*/
+
+ /**
+ * @backward-compat { version 146 }
+ *
+ * The `web-search` DOM node will not exist until Fx146+ so we have
+ * to fall back to the `search` DOM node. Adding this shim causes
+ * renderPreferenceSection to have too many statements. We can remove
+ * this eslint exception once we are in Fx146+.
+ */
+ // eslint-disable-next-line max-statements
renderPreferenceSection(sectionData, document, Preferences) {
+ /**
+ * @backward-compat { version 146 }
+ *
+ * We have to potentially re-assign the `id` if it is `web-search`.
+ * We should restore `id` back to a const after Fx146+.
+ */
+ let { id } = sectionData;
const {
- id,
pref: prefData,
maxRows,
rowsPref,
@@ -255,6 +271,19 @@ export class AboutPreferences {
return;
}
+ /**
+ * @backward-compat { version 146 }
+ *
+ * The `web-search` DOM node will not exist until Fx146+ so fall
+ * back to the `search` DOM node until then.
+ */
+ if (id === "web-search") {
+ let webSearchBox = document.getElementById(id);
+ if (!webSearchBox) {
+ id = "search";
+ }
+ }
+
// Add the main preference for turning on/off a section
const sectionVbox = document.getElementById(id);
sectionVbox.setAttribute("data-subcategory", id);
