morgan pushed to branch tor-browser-128.8.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 36916036 by Henry Wilkes at 2025-03-17T21:14:23+00:00 fixup! TB 27476: Implement about:torconnect captive portal within Tor Browser TB 43321: Only focus the about:torconnect buttons under certain circumstances. By default, when switching stages we move the focus back to the stage heading. This is because we want to lead the user back to the top of the page to show them the new context. This should help improve the experience when using a screen reader. If we are in the bootstrapping stage we instead move the focus to the "Cancel" button since it is likely that the user wants to use this control. If the user presses the "Cancel" button we return the focus to the "Connect" or "Try a bridge" button. I.e. we restore the prior focus. This allows to user to easily re-try without having to re-read the page they just saw. We do a similar thing when the user cancels the automatic startup bootstrapping. Finally, on page load we will focus the "Connect" button if the user has previously interacted with it. We record this interaction in a preference that persists between sessions. We also separate out the "Loading" stage from the "Start" stage. It is unexpected for `about:torconnect` to be opened whilst in the "Loading" stage, but if it does happen it would be safer to keep the page blank. The way this is implemented also ensures that the initial page is blank prior to "get-init-args" resolving. - - - - - ad21bdd6 by Henry Wilkes at 2025-03-17T21:14:23+00:00 fixup! TB 40597: Implement TorSettings module TB 43321: Add a isQuickstart property to the TorConnect.stage. This is used by `about:torconnect` for determining focus behaviour. - - - - - 5 changed files: - toolkit/components/torconnect/TorConnectParent.sys.mjs - toolkit/components/torconnect/content/aboutTorConnect.css - toolkit/components/torconnect/content/aboutTorConnect.html - toolkit/components/torconnect/content/aboutTorConnect.js - toolkit/modules/TorConnect.sys.mjs Changes: ===================================== toolkit/components/torconnect/TorConnectParent.sys.mjs ===================================== @@ -13,6 +13,9 @@ ChromeUtils.defineESModuleGetters(lazy, { HomePage: "resource:///modules/HomePage.sys.mjs", }); +const userHasEverClickedConnectPref = + "torbrowser.about_torconnect.user_has_ever_clicked_connect"; + /* This object is basically a marshalling interface between the TorConnect module and a particular about:torconnect page @@ -117,6 +120,9 @@ export class TorConnectParent extends JSWindowActorParent { TorConnect.chooseRegion(); break; case "torconnect:begin-bootstrapping": + if (message.data.userClickedConnect) { + Services.prefs.setBoolPref(userHasEverClickedConnectPref, true); + } TorConnect.beginBootstrapping(message.data.regionCode); break; case "torconnect:cancel-bootstrapping": @@ -130,6 +136,10 @@ export class TorConnectParent extends JSWindowActorParent { Direction: Services.locale.isAppLocaleRTL ? "rtl" : "ltr", CountryNames: TorConnect.countryNames, stage: TorConnect.stage, + userHasEverClickedConnect: Services.prefs.getBoolPref( + userHasEverClickedConnectPref, + false + ), quickstartEnabled: TorConnect.quickstart, }; case "torconnect:get-frequent-regions": ===================================== toolkit/components/torconnect/content/aboutTorConnect.css ===================================== @@ -15,6 +15,11 @@ html { height: 100%; } +body:not(.loaded) { + /* Keep blank whilst loading. */ + display: none; +} + #breadcrumbs { display: flex; align-items: center; @@ -93,6 +98,11 @@ html { display: none; } +#tor-connect-heading { + /* Do not show the focus outline. */ + outline: none; +} + #connect-to-tor { margin-inline-start: 0; } ===================================== toolkit/components/torconnect/content/aboutTorConnect.html ===================================== @@ -50,7 +50,7 @@ </div> <div id="text-container"> <div class="title"> - <h1 class="title-text"></h1> + <h1 id="tor-connect-heading" class="title-text" tabindex="-1"></h1> </div> <div id="connectLongContent"> <p id="connectLongContentText"></p> ===================================== toolkit/components/torconnect/content/aboutTorConnect.js ===================================== @@ -32,7 +32,6 @@ class AboutTorConnect { selectors = Object.freeze({ textContainer: { title: "div.title", - titleText: "h1.title-text", longContentText: "#connectLongContentText", }, progress: { @@ -77,7 +76,7 @@ class AboutTorConnect { elements = Object.freeze({ title: document.querySelector(this.selectors.textContainer.title), - titleText: document.querySelector(this.selectors.textContainer.titleText), + heading: document.getElementById("tor-connect-heading"), longContentText: document.querySelector( this.selectors.textContainer.longContentText ), @@ -138,18 +137,44 @@ class AboutTorConnect { locations = {}; - beginBootstrapping() { - RPMSendAsyncMessage("torconnect:begin-bootstrapping", {}); + /** + * Whether the user requested a cancellation of the bootstrap from *this* + * page. + * + * @type {boolean} + */ + userCancelled = false; + + /** + * Start a normal bootstrap attempt. + * + * @param {boolean} userClickedConnect - Whether this request was triggered by + * the user clicking the "Connect" button on the "Start" page. + */ + beginBootstrapping(userClickedConnect) { + RPMSendAsyncMessage("torconnect:begin-bootstrapping", { + userClickedConnect, + }); } + /** + * Start an auto bootstrap attempt. + * + * @param {string} regionCode - The region code to use for the bootstrap, or + * "automatic". + */ beginAutoBootstrapping(regionCode) { RPMSendAsyncMessage("torconnect:begin-bootstrapping", { regionCode, }); } + /** + * Try and cancel the current bootstrap attempt. + */ cancelBootstrapping() { RPMSendAsyncMessage("torconnect:cancel-bootstrapping"); + this.userCancelled = true; } /* @@ -260,7 +285,7 @@ class AboutTorConnect { } setTitle(title, className) { - this.elements.titleText.textContent = title; + this.elements.heading.textContent = title; this.elements.title.className = "title"; if (className) { this.elements.title.classList.add(className); @@ -349,18 +374,88 @@ class AboutTorConnect { } } + /** + * The connect button that was focused just prior to a bootstrap attempt, if + * any. + * + * @type {?Element} + */ + preBootstrappingFocus = null; + + /** + * The stage that was shown on this page just prior to a bootstrap attempt. + * + * @type {?string} + */ + preBootstrappingStage = null; + /* These methods update the UI based on the current TorConnect state */ - updateStage(stage) { + /** + * Update the shown stage. + * + * @param {ConnectStage} stage - The new stage to show. + * @param {boolean} [focusConnect=false] - Whether to try and focus the + * connect button, if we are in the Start stage. + */ + updateStage(stage, focusConnect = false) { if (stage.name === this.shownStage) { return; } + const prevStage = this.shownStage; this.shownStage = stage.name; this.selectedLocation = stage.defaultRegion; + // By default we want to reset the focus to the top of the page when + // changing the displayed page since we want a user to read the new page + // before activating a control. + let moveFocus = this.elements.heading; + + if (stage.name === "Bootstrapping") { + this.preBootstrappingStage = prevStage; + this.preBootstrappingFocus = null; + if (focusConnect && stage.isQuickstart) { + // If this is the initial automatic bootstrap triggered by the + // quickstart preference, treat as if the previous shown stage was + // "Start" and the user clicked the "Connect" button. + // Then, if the user cancels, the focus should still move to the + // "Connect" button. + this.preBootstrappingStage = "Start"; + this.preBootstrappingFocus = this.elements.connectButton; + } else if (this.elements.connectButton.contains(document.activeElement)) { + this.preBootstrappingFocus = this.elements.connectButton; + } else if ( + this.elements.tryBridgeButton.contains(document.activeElement) + ) { + this.preBootstrappingFocus = this.elements.tryBridgeButton; + } + } else { + if ( + this.userCancelled && + prevStage === "Bootstrapping" && + stage.name === this.preBootstrappingStage && + this.preBootstrappingFocus && + this.elements.cancelButton.contains(document.activeElement) + ) { + // If returning back to the same stage after the user tried to cancel + // bootstrapping from within this page, then we restore the focus to the + // connect button to allow the user to quickly re-try. + // If the bootstrap was cancelled for any other reason, we reset the + // focus as usual. + moveFocus = this.preBootstrappingFocus; + } + // Clear the Bootstrapping variables. + this.preBootstrappingStage = null; + this.preBootstrappingFocus = null; + } + + // Clear the recording of the cancellation request. + this.userCancelled = false; + + let isLoaded = true; let showProgress = false; let showLog = false; switch (stage.name) { @@ -368,14 +463,21 @@ class AboutTorConnect { console.error("Should not be open when TorConnect is disabled"); break; case "Loading": + // Unexpected for this page to open so early. + console.warn("Page opened whilst loading"); + isLoaded = false; + break; case "Start": - // Loading is not currnetly handled, treat the same as "Start", but UI - // will be unresponsive. this.showStart(stage.tryAgain, stage.potentiallyBlocked); + if (focusConnect) { + moveFocus = this.elements.connectButton; + } break; case "Bootstrapping": showProgress = true; this.showBootstrapping(stage.bootstrapTrigger, stage.tryAgain); + // Always focus the cancel button. + moveFocus = this.elements.cancelButton; break; case "Offline": showLog = true; @@ -419,6 +521,9 @@ class AboutTorConnect { } else { this.hide(this.elements.viewLogButton); } + + document.body.classList.toggle("loaded", isLoaded); + moveFocus.focus(); } updateBootstrappingStatus(data) { @@ -452,10 +557,9 @@ class AboutTorConnect { this.show(this.elements.quickstartContainer); this.show(this.elements.configureButton); this.show(this.elements.connectButton, true); - this.elements.connectButton.focus(); - if (tryAgain) { - this.elements.connectButton.textContent = TorStrings.torConnect.tryAgain; - } + this.elements.connectButton.textContent = tryAgain + ? TorStrings.torConnect.tryAgain + : TorStrings.torConnect.torConnectButton; if (potentiallyBlocked) { this.setBreadcrumbsStatus( BreadcrumbStatus.Active, @@ -511,7 +615,6 @@ class AboutTorConnect { } this.hideButtons(); this.show(this.elements.cancelButton); - this.elements.cancelButton.focus(); } showOffline() { @@ -541,7 +644,6 @@ class AboutTorConnect { BreadcrumbStatus.Disabled ); this.showLocationForm(true, TorStrings.torConnect.tryBridge); - this.elements.tryBridgeButton.focus(); } showRegionNotFound() { @@ -557,7 +659,6 @@ class AboutTorConnect { BreadcrumbStatus.Disabled ); this.showLocationForm(false, TorStrings.torConnect.tryBridge); - this.elements.tryBridgeButton.focus(); } showConfirmRegion(error) { @@ -573,7 +674,6 @@ class AboutTorConnect { BreadcrumbStatus.Active ); this.showLocationForm(false, TorStrings.torConnect.tryAgain); - this.elements.tryBridgeButton.focus(); } showFinalError(error) { @@ -722,7 +822,8 @@ class AboutTorConnect { this.elements.connectButton.textContent = TorStrings.torConnect.torConnectButton; this.elements.connectButton.addEventListener("click", () => { - this.beginBootstrapping(); + // Record as userClickedConnect if we are in the Start stage. + this.beginBootstrapping(this.shownStage === "Start"); }); this.populateLocations(); @@ -802,7 +903,13 @@ class AboutTorConnect { this.initObservers(); this.initKeyboardShortcuts(); - this.updateStage(args.stage); + // If we have previously opened about:torconnect and the user tried the + // "Connect" button we want to focus the "Connect" button for easy + // activation. + // Otherwise, we do not want to focus it for first time users so they can + // read the full page first. + const focusConnect = args.userHasEverClickedConnect; + this.updateStage(args.stage, focusConnect); this.updateQuickstart(args.quickstartEnabled); } } ===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -680,10 +680,14 @@ export const TorConnectStage = Object.freeze({ * A summary of the user stage. * (This class is mirrored for Android in TorConnectStage.java. Changes should be mirrored there) * - * @property {string} name - The name of the stage. + * @property {string} name - The name of the stage. One of the values in + * `TorConnectStage`. * @property {string} defaultRegion - The default region to show in the UI. - * @property {?string} bootstrapTrigger - The TorConnectStage prior to this + * @property {?string} bootstrapTrigger - The name of the stage prior to this * bootstrap attempt. Only set during the "Bootstrapping" stage. + * @property {boolean} isQuickstart - Whether the current bootstrap attempt was + * triggered by the `TorConnect.quickstart` preference. Will be `false` + * outside of the "Bootstrapping" stage. * @property {?BootstrapError} error - The last bootstrapping error. * @property {boolean} tryAgain - Whether a bootstrap attempt has failed, so * that a normal bootstrap should be shown as "Try Again" instead of @@ -752,6 +756,14 @@ export const TorConnect = { */ _bootstrapTrigger: null, + /** + * Whether the current bootstrapping attempt was triggered by the quickstart + * preference. + * + * @type {boolean} + */ + _isQuickstart: false, + /** * The alternative stage that we should move to after bootstrapping completes. * @@ -807,6 +819,7 @@ export const TorConnect = { name: this._stageName, defaultRegion: this._defaultRegion, bootstrapTrigger: this._bootstrapTrigger, + isQuickstart: this._isQuickstart, error: this._errorDetails ? { code: this._errorDetails.code, @@ -935,7 +948,7 @@ export const TorConnect = { // And the previous bootstrap attempt must have succeeded. !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) ) { - this.beginBootstrapping(); + this._beginBootstrappingInternal(undefined, true); } }, @@ -1303,6 +1316,19 @@ export const TorConnect = { * an auto-bootstrap attempt. */ async beginBootstrapping(regionCode) { + await this._beginBootstrappingInternal(regionCode, false); + }, + + /** + * Begin a bootstrap attempt. + * + * @param {string} [regionCode] - An optional region code string to use, or + * "automatic" to automatically determine the region. If given, will start + * an auto-bootstrap attempt. + * @param {boolean} isQuickstart - Whether this was triggered by the + * quickstart option. + */ + async _beginBootstrappingInternal(regionCode, isQuickstart) { lazy.logger.debug("TorConnect.beginBootstrapping()"); if (!this._confirmBootstrapping(regionCode)) { @@ -1331,6 +1357,7 @@ export const TorConnect = { } this._requestedStage = null; this._bootstrapTrigger = beginStage; + this._isQuickstart = isQuickstart; this._setStage(TorConnectStage.Bootstrapping); this._bootstrapAttempt = bootstrapAttempt; @@ -1349,6 +1376,7 @@ export const TorConnect = { const requestedStage = this._requestedStage; this._requestedStage = null; this._bootstrapTrigger = null; + this._isQuickstart = false; this._bootstrapAttempt = null; if (bootstrapAttempt.detectedRegion) { View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/f5c2815... -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/f5c2815... You're receiving this email because of your account on gitlab.torproject.org.