tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit a3ed2d55e95a2f80b0aca9c1e6df1ec23341a9fc
parent 8274119837796e046299777ba0b328bd390836c7
Author: Benjamin VanderSloot <bvandersloot@mozilla.com>
Date:   Mon, 29 Dec 2025 13:28:34 +0000

Bug 1988807 - Clean up duplicated markup in security-privacy-card with semantic html - r=mkennedy

This was initially going to be a new component for the bullets, but I
really think this removes a lot of the mess while leaving the right
amount of repitition in the markup. Using some CSS pseudo-classes and
semantic HTML makes it way better without the new interface to maintain

Differential Revision: https://phabricator.services.mozilla.com/D275504

Diffstat:
Mbrowser/components/preferences/privacy.js | 288+++++++++++++++++++++++++++++++++++++++----------------------------------------
Mbrowser/components/preferences/tests/browser_privacy_status_card.js | 35+++++++++++------------------------
Mbrowser/components/preferences/widgets/security-privacy/security-privacy-card/security-privacy-card.css | 52+++++++++++++++++++++++++++++++---------------------
Mbrowser/components/preferences/widgets/security-privacy/security-privacy-card/security-privacy-card.mjs | 129+++++++++++++++++++++++++++----------------------------------------------------
4 files changed, 228 insertions(+), 276 deletions(-)

diff --git a/browser/components/preferences/privacy.js b/browser/components/preferences/privacy.js @@ -119,6 +119,13 @@ const SANITIZE_ON_SHUTDOWN_PREFS_ONLY_V2 = [ "privacy.clearOnShutdown_v2.siteSettings", ]; +const SECURITY_PRIVACY_STATUS_CARD_ENABLED = + Services.prefs.getBoolPref("browser.settings-redesign.enabled", false) || + Services.prefs.getBoolPref( + "browser.settings-redesign.securityPrivacyStatus.enabled", + false + ); + Preferences.addAll([ // Content blocking / Tracking Protection { id: "privacy.trackingprotection.enabled", type: "bool" }, @@ -287,7 +294,7 @@ Preferences.addAll([ { id: "media.setsinkid.enabled", type: "bool" }, ]); -if (Services.prefs.getBoolPref("privacy.ui.status_card", false)) { +if (SECURITY_PRIVACY_STATUS_CARD_ENABLED) { Preferences.addAll([ // Security and Privacy Warnings { id: "privacy.ui.status_card.testing.show_issue", type: "bool" }, @@ -352,7 +359,7 @@ if (Services.prefs.getBoolPref("privacy.ui.status_card", false)) { type: "bool", }, { - id: "browser.preferences.config_warning.warningPrivelegedConstraint.dismissed", + id: "browser.preferences.config_warning.warningPrivilegedConstraint.dismissed", type: "bool", }, { @@ -1156,7 +1163,7 @@ if (SECURITY_PRIVACY_STATUS_CARD_ENABLED) { Preferences.addSetting( new WarningSettingConfig( - "warningPrivelegedConstraint", + "warningPrivilegedConstraint", { rsl: "security.disallow_privilegedabout_remote_script_loads", shfa: "dom.security.skip_html_fragment_assertion", @@ -1267,53 +1274,139 @@ if (SECURITY_PRIVACY_STATUS_CARD_ENABLED) { true ) ); -} -/** @type {SettingControlConfig[]} */ -const SECURITY_WARNINGS = [ - { - l10nId: "security-privacy-issue-warning-test", - id: "warningTest", - }, - { - l10nId: "security-privacy-issue-warning-fingerprinters", - id: "warningAllowFingerprinters", - }, - { - l10nId: "security-privacy-issue-warning-third-party-cookies", - id: "warningThirdPartyCookies", - }, - { - l10nId: "security-privacy-issue-warning-password-manager", - id: "warningPasswordManager", - }, - { - l10nId: "security-privacy-issue-warning-popup-blocker", - id: "warningPopupBlocker", - }, - { - l10nId: "security-privacy-issue-warning-extension-install", - id: "warningExtensionInstall", - }, - { - l10nId: "security-privacy-issue-warning-safe-browsing", - id: "warningSafeBrowsing", - }, - { - l10nId: "security-privacy-issue-warning-doh", - id: "warningDoH", - }, - { - l10nId: "security-privacy-issue-warning-ech", - id: "warningECH", - }, - { - l10nId: "security-privacy-issue-warning-ct", - id: "warningCT", - }, - { - l10nId: "security-privacy-issue-warning-crlite", - id: "warningCRLite", + /** @type {SettingControlConfig[]} */ + const SECURITY_WARNINGS = [ + { + l10nId: "security-privacy-issue-warning-test", + id: "warningTest", + }, + { + l10nId: "security-privacy-issue-warning-fingerprinters", + id: "warningAllowFingerprinters", + }, + { + l10nId: "security-privacy-issue-warning-third-party-cookies", + id: "warningThirdPartyCookies", + }, + { + l10nId: "security-privacy-issue-warning-password-manager", + id: "warningPasswordManager", + }, + { + l10nId: "security-privacy-issue-warning-popup-blocker", + id: "warningPopupBlocker", + }, + { + l10nId: "security-privacy-issue-warning-extension-install", + id: "warningExtensionInstall", + }, + { + l10nId: "security-privacy-issue-warning-safe-browsing", + id: "warningSafeBrowsing", + }, + { + l10nId: "security-privacy-issue-warning-doh", + id: "warningDoH", + }, + { + l10nId: "security-privacy-issue-warning-ech", + id: "warningECH", + }, + { + l10nId: "security-privacy-issue-warning-ct", + id: "warningCT", + }, + { + l10nId: "security-privacy-issue-warning-crlite", + id: "warningCRLite", + }, + { + l10nId: "security-privacy-issue-warning-certificate-pinning", + id: "warningCertificatePinning", + }, + { + l10nId: "security-privacy-issue-warning-tlsmin", + id: "warningTLSMin", + }, + { + l10nId: "security-privacy-issue-warning-tlsmax", + id: "warningTLSMax", + }, + { + l10nId: "security-privacy-issue-warning-proxy-autodetection", + id: "warningProxyAutodetection", + }, + { + l10nId: "security-privacy-issue-warning-content-resource-uri", + id: "warningContentResourceURI", + }, + { + l10nId: "security-privacy-issue-warning-worker-mime", + id: "warningWorkerMIME", + }, + { + l10nId: "security-privacy-issue-warning-top-level-data-uri", + id: "warningTopLevelDataURI", + }, + { + l10nId: "security-privacy-issue-warning-active-mixed-content", + id: "warningActiveMixedContent", + }, + { + l10nId: "security-privacy-issue-warning-inner-html-ltgt", + id: "warningInnerHTMLltgt", + }, + { + l10nId: "security-privacy-issue-warning-file-uri-origin", + id: "warningFileURIOrigin", + }, + { + l10nId: "security-privacy-issue-warning-privileged-constraint", + id: "warningPrivilegedConstraint", + }, + { + l10nId: "security-privacy-issue-warning-process-sandbox", + id: "warningProcessSandbox", + }, + ]; + + Preferences.addSetting( + /** @type {{ makeSecurityWarningItems: () => SettingControlConfig[] } & SettingConfig} */ ({ + id: "securityWarningsGroup", + makeSecurityWarningItems() { + return SECURITY_WARNINGS.map(({ id, l10nId }) => ({ + id, + l10nId, + control: "moz-box-item", + options: [ + { + control: "moz-button", + l10nId: "issue-card-reset-button", + controlAttrs: { slot: "actions", size: "small", id: "reset" }, + }, + { + control: "moz-button", + l10nId: "issue-card-dismiss-button", + controlAttrs: { + slot: "actions", + size: "small", + iconsrc: "chrome://global/skin/icons/close.svg", + id: "dismiss", + }, + }, + ], + })); + }, + getControlConfig(config) { + if (!config.items) { + return { ...config, items: this.makeSecurityWarningItems() }; + } + return config; + }, + }) + ); + Preferences.addSetting({ id: "privacyCard", deps: [ @@ -1332,103 +1425,6 @@ const SECURITY_WARNINGS = [ }, }); } - id: "warningCertificatePinning", - }, - { - l10nId: "security-privacy-issue-warning-tlsmin", - id: "warningTLSMin", - }, - { - l10nId: "security-privacy-issue-warning-tlsmax", - id: "warningTLSMax", - }, - { - l10nId: "security-privacy-issue-warning-proxy-autodetection", - id: "warningProxyAutodetection", - }, - { - l10nId: "security-privacy-issue-warning-content-resource-uri", - id: "warningContentResourceURI", - }, - { - l10nId: "security-privacy-issue-warning-worker-mime", - id: "warningWorkerMIME", - }, - { - l10nId: "security-privacy-issue-warning-top-level-data-uri", - id: "warningTopLevelDataURI", - }, - { - l10nId: "security-privacy-issue-warning-active-mixed-content", - id: "warningActiveMixedContent", - }, - { - l10nId: "security-privacy-issue-warning-inner-html-ltgt", - id: "warningInnerHTMLltgt", - }, - { - l10nId: "security-privacy-issue-warning-file-uri-origin", - id: "warningFileURIOrigin", - }, - { - l10nId: "security-privacy-issue-warning-priveleged-constraint", - id: "warningPrivelegedConstraint", - }, - { - l10nId: "security-privacy-issue-warning-process-sandbox", - id: "warningProcessSandbox", - }, -]; - -Preferences.addSetting( - /** @type {{ makeSecurityWarningItems: () => SettingControlConfig[] } & SettingConfig} */ ({ - id: "securityWarningsGroup", - makeSecurityWarningItems() { - return SECURITY_WARNINGS.map(({ id, l10nId }) => ({ - id, - l10nId, - control: "moz-box-item", - options: [ - { - control: "moz-button", - l10nId: "issue-card-reset-button", - controlAttrs: { slot: "actions", size: "small", id: "reset" }, - }, - { - control: "moz-button", - l10nId: "issue-card-dismiss-button", - controlAttrs: { - slot: "actions", - size: "small", - iconsrc: "chrome://global/skin/icons/close.svg", - id: "dismiss", - }, - }, - ], - })); - }, - getControlConfig(config) { - if (!config.items) { - return { ...config, items: this.makeSecurityWarningItems() }; - } - return config; - }, - }) -); - -Preferences.addSetting({ - id: "privacyCard", - deps: [ - "appUpdateStatus", - "trackerCount", - "etpStrictEnabled", - ...SECURITY_WARNINGS.map(warning => warning.id), - ], -}); - -Preferences.addSetting({ - id: "warningCard", -}); Preferences.addSetting({ id: "ipProtectionVisible", diff --git a/browser/components/preferences/tests/browser_privacy_status_card.js b/browser/components/preferences/tests/browser_privacy_status_card.js @@ -3,7 +3,7 @@ "use strict"; -const FEATURE_PREF = "privacy.ui.status_card"; +const FEATURE_PREF = "browser.settings-redesign.enabled"; const CARD_NAME = "security-privacy-card"; const ISSUE_CONTROL_ID = "securityWarningsGroup"; @@ -71,11 +71,11 @@ function getCardAndCheckHeader(document, expectedHeaderL10n) { } function assertHappyBullets(card) { - let bullets = card.shadowRoot.querySelectorAll(".status-bullet > img"); + let bullets = card.shadowRoot.querySelectorAll("li"); Assert.equal(bullets.length, 2); for (const bullet of bullets) { Assert.equal( - bullet.classList.contains("check-bullet"), + bullet.classList.contains("status-ok"), true, "All bullets must be happy!" ); @@ -180,15 +180,12 @@ add_task(async function test_issue_present() { browser.contentDocument, "security-privacy-status-problem-header" ); - let bulletIcons = card.shadowRoot.querySelectorAll( - ".status-bullet > img" - ); + let bulletIcons = card.shadowRoot.querySelectorAll("li"); Assert.equal(bulletIcons.length, 2); let problemsBulletIcon = bulletIcons[0]; - Assert.ok(problemsBulletIcon.classList.contains("alert-bullet")); - let bulletLink = card.shadowRoot.querySelector(".status-bullet"); + Assert.ok(problemsBulletIcon.classList.contains("status-alert")); Assert.notEqual( - bulletLink.querySelector("a"), + problemsBulletIcon.querySelector("a"), null, "Link to issues is present" ); @@ -302,15 +299,11 @@ add_task(async function test_update_status_indicator() { // Define testers for each UI state. let absent = card => { - let label = card.shadowRoot.querySelector( - "div.status-bullet:nth-child(4) > .status-label-holder > div" - ); + let label = card.shadowRoot.querySelector("li:nth-child(3) p"); Assert.equal(label, null, "No install status label is present"); }; let issue = card => { - let label = card.shadowRoot.querySelector( - "div.status-bullet:nth-child(4) > .status-label-holder > div" - ); + let label = card.shadowRoot.querySelector("li:nth-child(3) p"); Assert.equal( label.attributes.getNamedItem("data-l10n-id").value, "security-privacy-status-update-error-label", @@ -318,9 +311,7 @@ add_task(async function test_update_status_indicator() { ); }; let needed = card => { - let label = card.shadowRoot.querySelector( - "div.status-bullet:nth-child(4) > .status-label-holder > div" - ); + let label = card.shadowRoot.querySelector("li:nth-child(3) p"); Assert.equal( label.attributes.getNamedItem("data-l10n-id").value, "security-privacy-status-update-needed-label", @@ -328,9 +319,7 @@ add_task(async function test_update_status_indicator() { ); }; let ok = card => { - let label = card.shadowRoot.querySelector( - "div.status-bullet:nth-child(4) > div" - ); + let label = card.shadowRoot.querySelector("li:nth-child(3) p"); Assert.equal( label.attributes.getNamedItem("data-l10n-id").value, "security-privacy-status-up-to-date-label", @@ -338,9 +327,7 @@ add_task(async function test_update_status_indicator() { ); }; let checking = card => { - let label = card.shadowRoot.querySelector( - "div.status-bullet:nth-child(4) > div" - ); + let label = card.shadowRoot.querySelector("li:nth-child(3) p"); Assert.equal( label.attributes.getNamedItem("data-l10n-id").value, "security-privacy-status-update-checking-label", diff --git a/browser/components/preferences/widgets/security-privacy/security-privacy-card/security-privacy-card.css b/browser/components/preferences/widgets/security-privacy/security-privacy-card/security-privacy-card.css @@ -2,49 +2,58 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -.check-bullet, -.alert-bullet, -.throbber-bullet { +.status-ok::before, +.status-alert::before, +.status-loading::before { -moz-context-properties: fill; - display: inline-block; - vertical-align: text-top; height: var(--icon-size); width: var(--icon-size); margin-inline-end: var(--space-medium); } -.check-bullet { + +.status-ok::before { + content: url("chrome://global/skin/icons/check-filled.svg"); fill: var(--icon-color-success); } -.alert-bullet { +.status-alert::before { + content: url("chrome://global/skin/icons/warning.svg"); fill: var(--icon-color-warning); } +.status-loading::before { + content: url("chrome://global/skin/icons/loading.svg"); + fill: currentColor; +} + small { font-size: var(--font-size-small); color: var(--text-color-deemphasized); } -.status-container { +.card-contents { display: flex; } -.status-bullet-container { - display: flex; +.status-text-container { flex-grow: 1; +} + +ul { + display: flex; flex-direction: column; gap: var(--space-medium); + list-style-type: none; + margin-block: var(--space-medium) 0; + padding: 0; } -.status-bullet { +li { display: flex; - margin: 0; } -.status-label-holder { - display: flex; - flex-direction: column; - width: 100%; +p { + margin: 0; } a { @@ -57,13 +66,10 @@ a:hover:active { text-decoration: none; } -#heading { +h3 { font-size: var(--font-size-root); font-weight: var(--heading-font-weight); -} - -moz-card { - margin-block-end: var(--space-xxlarge); + margin: 0; } .status-label-holder > moz-box-link { @@ -74,3 +80,7 @@ moz-card { margin-inline-start: var(--space-xsmall); max-width: 100px; } + +.status-image:dir(rtl) { + transform: scaleX(-1); +} diff --git a/browser/components/preferences/widgets/security-privacy/security-privacy-card/security-privacy-card.mjs b/browser/components/preferences/widgets/security-privacy/security-privacy-card/security-privacy-card.mjs @@ -144,25 +144,14 @@ export default class SecurityPrivacyCard extends MozLitElement { */ buildIssuesElement() { if (this.configIssueCount == 0) { - return html`<div class="status-bullet"> - <img - class="check-bullet" - src="chrome://global/skin/icons/check-filled.svg" - alt="status ok" - /> - <div data-l10n-id=${L10N_IDS.okLabel}></div> - </div>`; + return html`<li class="status-ok"> + <p data-l10n-id=${L10N_IDS.okLabel}></p> + </li>`; } - return html`<div class="status-bullet"> - <img - class="alert-bullet" - src="chrome://global/skin/icons/warning.svg" - alt="status warning" - /> - - <div class="status-label-holder"> - <div data-l10n-id=${L10N_IDS.problemLabel}></div> - <div> + return html`<li class="status-alert"> + <div> + <p data-l10n-id=${L10N_IDS.problemLabel}></p> + <p> <small ><a href="" @@ -170,9 +159,9 @@ export default class SecurityPrivacyCard extends MozLitElement { data-l10n-id=${L10N_IDS.problemHelperLabel} ></a ></small> - </div> + </p> </div> - </div>`; + </li>`; } /** @@ -187,22 +176,17 @@ export default class SecurityPrivacyCard extends MozLitElement { }; let trackerLabelElement = this.trackersBlocked != null - ? html`<div + ? html`<p data-l10n-id=${L10N_IDS.trackersLabel} data-l10n-args=${JSON.stringify(trackerData)} - ></div>` - : html`<div data-l10n-id=${L10N_IDS.trackersPendingLabel}></div>`; + ></p>` + : html`<p data-l10n-id=${L10N_IDS.trackersPendingLabel}></p>`; if (this.strictEnabled) { - return html`<div class="status-bullet"> - <img - class="check-bullet" - src="chrome://global/skin/icons/check-filled.svg" - alt="status ok" - /> - <div class="status-label-holder"> + return html`<li class="status-ok"> + <div> ${trackerLabelElement} - <div> + <p> <small data-l10n-id=${L10N_IDS.strictEnabledLabel} id="strictEnabled" @@ -210,18 +194,11 @@ export default class SecurityPrivacyCard extends MozLitElement { > <a data-l10n-name="strict-tracking-protection" href=""></a ></small> - </div> + </p> </div> - </div>`; + </li>`; } - return html`<div class="status-bullet"> - <img - class="check-bullet" - src="chrome://global/skin/icons/check-filled.svg" - alt="status ok" - /> - ${trackerLabelElement} - </div>`; + return html`<li class="status-ok">${trackerLabelElement}</li>`; } /** @@ -233,72 +210,52 @@ export default class SecurityPrivacyCard extends MozLitElement { buildUpdateElement() { switch (this.appUpdateStatus) { case lazy.AppUpdater.STATUS.NO_UPDATES_FOUND: - return html`<div class="status-bullet"> - <img - class="check-bullet" - src="chrome://global/skin/icons/check-filled.svg" - alt="status ok" - /> - <div data-l10n-id=${L10N_IDS.upToDateLabel}></div> - </div>`; + return html`<li class="status-ok"> + <p data-l10n-id=${L10N_IDS.upToDateLabel}></p> + </li>`; case lazy.AppUpdater.STATUS.MANUAL_UPDATE: case lazy.AppUpdater.STATUS.DOWNLOADING: case lazy.AppUpdater.STATUS.DOWNLOAD_AND_INSTALL: case lazy.AppUpdater.STATUS.STAGING: case lazy.AppUpdater.STATUS.READY_FOR_RESTART: - return html`<div class="status-bullet"> - <img - class="alert-bullet" - src="chrome://global/skin/icons/warning.svg" - alt="status warning" - /> - <div class="status-label-holder"> - <div data-l10n-id=${L10N_IDS.updateNeededLabel}></div> - <div> + return html`<li class="status-alert"> + <div> + <p data-l10n-id=${L10N_IDS.updateNeededLabel}></p> + <p> <small ><span data-l10n-id=${L10N_IDS.updateNeededDescription}></span ></small> - </div> + </p> <moz-box-link @click=${this.#scrollToTargetOnPanel("#general", "updateApp")} data-l10n-id=${L10N_IDS.updateButtonLabel} ></moz-box-link> </div> - </div>`; + </li>`; case lazy.AppUpdater.STATUS.NEVER_CHECKED: case lazy.AppUpdater.STATUS.UNSUPPORTED_SYSTEM: case lazy.AppUpdater.STATUS.DOWNLOAD_FAILED: case lazy.AppUpdater.STATUS.INTERNAL_ERROR: case lazy.AppUpdater.STATUS.CHECKING_FAILED: - return html`<div class="status-bullet"> - <img - class="alert-bullet" - src="chrome://global/skin/icons/warning.svg" - alt="status warning" - /> - <div class="status-label-holder"> - <div data-l10n-id=${L10N_IDS.updateErrorLabel}></div> - <div> + return html`<li class="status-alert"> + <div> + <p data-l10n-id=${L10N_IDS.updateErrorLabel}></p> + <p> <small ><span data-l10n-id=${L10N_IDS.updateNeededDescription}></span ></small> - </div> + </p> <moz-box-link href="javascript:void(0)" @click=${this.#scrollToTargetOnPanel("#general", "updateApp")} data-l10n-id=${L10N_IDS.updateButtonLabel} ></moz-box-link> </div> - </div>`; + </li>`; case lazy.AppUpdater.STATUS.CHECKING: - return html`<div class="status-bullet"> - <img - class="throbber-bullet" - src="chrome://global/skin/icons/loading.svg" - alt="status loading" - /> - <div data-l10n-id=${L10N_IDS.updateCheckingLabel}></div> - </div>`; + return html`<li class="status-loading"> + <p data-l10n-id=${L10N_IDS.updateCheckingLabel}></p> + </li>`; case lazy.AppUpdater.STATUS.NO_UPDATER: case lazy.AppUpdater.STATUS.UPDATE_DISABLED_BY_POLICY: case lazy.AppUpdater.STATUS.OTHER_INSTANCE_HANDLING_UPDATES: @@ -333,15 +290,17 @@ export default class SecurityPrivacyCard extends MozLitElement { href="chrome://browser/content/preferences/widgets/security-privacy-card.css" /> <moz-card aria-labelledby="heading"> - <div class="status-container"> - <div class="status-bullet-container"> - <div + <div class="card-contents"> + <div class="status-text-container"> + <h3 id="heading" data-l10n-id=${headerL10nId} data-l10n-args=${JSON.stringify(headerL10nData)} - ></div> - ${this.buildIssuesElement()} ${this.buildTrackersElement()} - ${this.buildUpdateElement()} + ></h3> + <ul> + ${this.buildIssuesElement()} ${this.buildTrackersElement()} + ${this.buildUpdateElement()} + </ul> </div> ${this.getStatusImage()} </div>