fix(portfolio): keep the balance out of the aria labels in privacy mode

CHANGELOG-Nns-Dapp-unreleased.md

@@ -38,6 +38,10 @@ proposal is successful, the changes it released will be moved from this file to
 
 #### Security
 
+- The "Hide Balance" option now also masks the balances in the accessible names
+  on the Portfolio page. Before, the cards kept the exact amounts in their
+  `aria-label` attributes, so a screen reader announced them.
+
 #### Not Published
 
 ### Operations

frontend/src/lib/components/portfolio/HeldTokensCard.svelte

@@ -6,6 +6,7 @@
   import { OWN_CANISTER_ID_TEXT } from "$lib/constants/canister-ids.constants";
   import { PRICE_NOT_AVAILABLE_PLACEHOLDER } from "$lib/constants/constants";
   import { AppPath } from "$lib/constants/routes.constants";
+  import { isBalancePrivacyOptionStore } from "$lib/derived/balance-privacy-active.derived";
   import { isDesktopViewportStore } from "$lib/derived/viewport.derived";
   import { i18n } from "$lib/stores/i18n";
   import type { UserTokenData } from "$lib/types/tokens-page";
@@ -106,7 +107,11 @@
         class="balance-usd"
         data-tid="balance-in-usd"
         role="cell"
-        aria-label={`${token.title} USD: ${token?.balanceInUsd ?? 0}`}
+        aria-label={`${token.title} USD: ${
+          $isBalancePrivacyOptionStore
+            ? $i18n.portfolio.hidden_balance_label
+            : (token?.balanceInUsd ?? 0)
+        }`}
       >
         $<PrivacyAwareAmount
           value={formatNumber(token?.balanceInUsd ?? 0)}

frontend/src/lib/components/portfolio/StakedTokensCard.svelte

@@ -10,6 +10,7 @@
   import { PRICE_NOT_AVAILABLE_PLACEHOLDER } from "$lib/constants/constants";
   import { AppPath } from "$lib/constants/routes.constants";
   import { authSignedInStore } from "$lib/derived/auth.derived";
+  import { isBalancePrivacyOptionStore } from "$lib/derived/balance-privacy-active.derived";
   import {
     isDesktopViewportStore,
     isMobileViewportStore,
@@ -125,6 +126,13 @@
 {#snippet row({ stakedToken }: { stakedToken: TableProject | undefined })}
   {@const apy = stakedToken?.apy}
   {#if nonNullish(stakedToken)}
+    {@const stakeNative =
+      stakedToken.stake instanceof TokenAmountV2
+        ? formatTokenV2({
+            value: stakedToken.stake,
+            detailed: false,
+          })
+        : PRICE_NOT_AVAILABLE_PLACEHOLDER}
     <a
       href={stakedToken.rowHref}
       class="row"
@@ -163,7 +171,11 @@
         class="stake-usd"
         data-tid="stake-in-usd"
         role="cell"
-        aria-label={`${stakedToken.title} USD: ${stakedToken?.stakeInUsd ?? 0}`}
+        aria-label={`${stakedToken.title} USD: ${
+          $isBalancePrivacyOptionStore
+            ? $i18n.portfolio.hidden_balance_label
+            : (stakedToken?.stakeInUsd ?? 0)
+        }`}
       >
         $<PrivacyAwareAmount
           value={formatNumber(stakedToken?.stakeInUsd ?? 0)}
@@ -174,17 +186,13 @@
         class="stake-native"
         data-tid="stake-in-native"
         role="cell"
-        aria-label={`${stakedToken.title} D: ${stakedToken?.stakeInUsd ?? 0}`}
+        aria-label={`${stakedToken.title} ${stakedToken.stake.token.symbol}: ${
+          $isBalancePrivacyOptionStore
+            ? $i18n.portfolio.hidden_balance_label
+            : stakeNative
+        }`}
       >
-        <PrivacyAwareAmount
-          value={stakedToken.stake instanceof TokenAmountV2
-            ? formatTokenV2({
-                value: stakedToken.stake,
-                detailed: false,
-              })
-            : PRICE_NOT_AVAILABLE_PLACEHOLDER}
-          length={3}
-        />
+        <PrivacyAwareAmount value={stakeNative} length={3} />
         {stakedToken.stake.token.symbol}
       </div>
     </a>

frontend/src/lib/components/portfolio/TokensCardHeader.svelte

@@ -2,7 +2,9 @@
   import PrivacyAwareAmount from "$lib/components/ui/PrivacyAwareAmount.svelte";
   import { PRICE_NOT_AVAILABLE_PLACEHOLDER } from "$lib/constants/constants";
   import { authSignedInStore } from "$lib/derived/auth.derived";
+  import { isBalancePrivacyOptionStore } from "$lib/derived/balance-privacy-active.derived";
   import { isMobileViewportStore } from "$lib/derived/viewport.derived";
+  import { i18n } from "$lib/stores/i18n";
   import { formatCurrencyNumber } from "$lib/utils/format.utils";
   import { IconRight } from "@dfinity/gix-components";
   import { nonNullish } from "@dfinity/utils";
@@ -32,7 +34,15 @@
     </div>
     <div class="text-content">
       <h5 class="title">{title}</h5>
-      <p class="amount" data-tid="amount" aria-label={`${title}: ${usdAmount}`}>
+      <p
+        class="amount"
+        data-tid="amount"
+        aria-label={`${title}: ${
+          $isBalancePrivacyOptionStore
+            ? $i18n.portfolio.hidden_balance_label
+            : usdAmount
+        }`}
+      >
         $<PrivacyAwareAmount value={usdAmountFormatted} length={3} />
       </p>
     </div>

frontend/src/lib/i18n/en.json

@@ -1391,6 +1391,7 @@
     "login_description": "Sign in and start staking to earn voting rewards and participate in the Internet Computer’s onchain governance.",
     "no_tokens_card_description": "Store your ICP, ckTokens, and SNS governance tokens safely in the NNS wallet, hosted 100% on the Internet Computer blockchain – sign in to see tokens.",
     "no_neurons_card_description": "Stake your ICP and SNS tokens in neurons to participate in governance and gain voting rewards – sign in to stake your neurons.",
+    "hidden_balance_label": "hidden",
     "held_icp_card_title": "ICP Balance",
     "held_tokens_card_title": "Tokens Balance",
     "held_icp_card_link": "View accounts",

frontend/src/lib/types/i18n.d.ts

@@ -1464,6 +1464,7 @@ interface I18nPortfolio {
   login_description: string;
   no_tokens_card_description: string;
   no_neurons_card_description: string;
+  hidden_balance_label: string;
   held_icp_card_title: string;
   held_tokens_card_title: string;
   held_icp_card_link: string;

frontend/src/tests/e2e/portfolio-privacy-aria-labels.spec.ts

@@ -0,0 +1,152 @@
+/**
+ * The "Hide Balance" option must keep the amounts out of the accessibility
+ * tree, and it must still leave every cell with a usable accessible name.
+ *
+ * The Portfolio cards mask the amounts on screen with PrivacyAwareAmount.
+ * PrivacyAwareAmount masks the element content only. An aria-label on the same
+ * element replaces that content as the accessible name, so the label must
+ * follow the same store as the visible value.
+ *
+ * This test drives the real application and reads the accessible names in both
+ * states.
+ */
+import { AppPo } from "$tests/page-objects/App.page-object";
+import { PlaywrightPageObjectElement } from "$tests/page-objects/playwright.page-object";
+import { signInWithNewUser, step } from "$tests/utils/e2e.test-utils";
+import { expect, test } from "@playwright/test";
+
+// A Playwright spec cannot import $lib/i18n/en.json, because ESM asks for an
+// import attribute of type json. The expected strings are copied here.
+const HELD_ICP_CARD_TITLE = "ICP Balance";
+const STAKED_ICP_CARD_TITLE = "ICP Staking Balance";
+const HIDDEN_BALANCE_LABEL = "hidden";
+
+const DIGIT = /\d/;
+
+// playwright.config.ts sets expect.timeout to 0, so every poll needs its own
+// timeout. Without one a failing poll hangs until the 300 s test timeout.
+const POLL = { timeout: 30_000 };
+
+test("Privacy mode keeps the balances out of the Portfolio accessible names", async ({
+  page,
+  browser,
+}) => {
+  await page.goto("/");
+  await expect(page).toHaveTitle("Portfolio | Network Nervous System");
+
+  const pageElement = PlaywrightPageObjectElement.fromPage(page);
+  const appPo = new AppPo(pageElement);
+
+  step("Sign in");
+  await signInWithNewUser({ page, context: browser.contexts()[0] });
+
+  step("Get some ICP");
+  await appPo.getIcpTokens(20);
+
+  step("Stake a neuron, so the staked tokens card has a row");
+  await appPo.goToStaking();
+  await appPo
+    .getStakingPo()
+    .stakeFirstNnsNeuron({ amount: 10, dissolveDelayDays: "max" });
+  await appPo.getNeuronsPo().waitFor();
+
+  step("Open the Portfolio page");
+  await page.goto("/");
+  const portfolioPagePo = appPo.getPortfolioPo().getPortfolioPagePo();
+  const heldCardPo = portfolioPagePo.getHeldICPCardPo();
+  const stakedCardPo = portfolioPagePo.getStakedICPCardPo();
+  await heldCardPo.waitFor();
+  await stakedCardPo.waitFor();
+
+  // Every accessible name that carries a balance on the Portfolio page.
+  const getBalanceAriaLabels = async (): Promise<(string | null)[]> => [
+    ...(await heldCardPo.getHeldTokensBalanceInUsdAriaLabels()),
+    await heldCardPo.getAmountAriaLabel(),
+    ...(await stakedCardPo.getStakedTokensStakeInUsdAriaLabels()),
+    ...(await stakedCardPo.getStakedTokensStakeInNativeCurrencyAriaLabels()),
+    await stakedCardPo.getAmountAriaLabel(),
+  ];
+
+  // Every attribute inside the two cards that a screen reader can use as an
+  // accessible name.
+  const getAllAccessibleNamesInCards = (): Promise<string[]> =>
+    page
+      .locator(
+        [
+          '[data-tid="held-icp-card"] [aria-label]',
+          '[data-tid="held-icp-card"] [title]',
+          '[data-tid="held-icp-card"] [alt]',
+          '[data-tid="staked-icp-card"] [aria-label]',
+          '[data-tid="staked-icp-card"] [title]',
+          '[data-tid="staked-icp-card"] [alt]',
+        ].join(", ")
+      )
+      .evaluateAll((elements) =>
+        elements.flatMap((element) =>
+          ["aria-label", "title", "alt"]
+            .map((attribute) => element.getAttribute(attribute))
+            .filter((value): value is string => value !== null)
+        )
+      );
+
+  const toggleBalancePrivacy = async () => {
+    // The Popover backdrop closes on click or on Enter/Space, not Escape
+    // (gix-components handleKeyPress only handles those two keys). The
+    // backdrop covers the whole screen, including the account-menu button
+    // itself, so a click on "backdrop" is the reliable way to close it.
+    const accountMenuPo = appPo.getAccountMenuPo();
+    await accountMenuPo.openMenu();
+    await accountMenuPo.getToggleBalancePrivacyOptionPo().click();
+    await accountMenuPo.click("backdrop");
+    await expect.poll(() => accountMenuPo.isOpen(), POLL).toBe(false);
+  };
+
+  step("Privacy mode off: every accessible name carries its amount");
+  await expect
+    .poll(getBalanceAriaLabels, POLL)
+    .toEqual([
+      expect.stringMatching(/^Internet Computer USD: \d/),
+      expect.stringMatching(new RegExp(`^${HELD_ICP_CARD_TITLE}: \\d`)),
+      expect.stringMatching(/^Internet Computer USD: \d/),
+      expect.stringMatching(/^Internet Computer ICP: \d/),
+      expect.stringMatching(new RegExp(`^${STAKED_ICP_CARD_TITLE}: \\d`)),
+    ]);
+
+  step("Turn privacy mode on");
+  await toggleBalancePrivacy();
+
+  step("Privacy mode on: the amounts are masked on screen");
+  await expect.poll(() => heldCardPo.getAmount(), POLL).toContain("•");
+  await expect.poll(() => stakedCardPo.getAmount(), POLL).toContain("•");
+
+  step("Privacy mode on: no accessible name carries an amount");
+  // Each name still says which token and which unit the cell holds, so a
+  // screen reader user can still tell the cells apart.
+  await expect
+    .poll(getBalanceAriaLabels, POLL)
+    .toEqual([
+      `Internet Computer USD: ${HIDDEN_BALANCE_LABEL}`,
+      `${HELD_ICP_CARD_TITLE}: ${HIDDEN_BALANCE_LABEL}`,
+      `Internet Computer USD: ${HIDDEN_BALANCE_LABEL}`,
+      `Internet Computer ICP: ${HIDDEN_BALANCE_LABEL}`,
+      `${STAKED_ICP_CARD_TITLE}: ${HIDDEN_BALANCE_LABEL}`,
+    ]);
+
+  step("Privacy mode on: no attribute in either card holds a digit");
+  const namesWithADigit = (await getAllAccessibleNamesInCards()).filter(
+    (name) => DIGIT.test(name)
+  );
+  expect(namesWithADigit).toEqual([]);
+
+  step("Turn privacy mode off again: the amounts come back");
+  await toggleBalancePrivacy();
+  await expect
+    .poll(getBalanceAriaLabels, POLL)
+    .toEqual([
+      expect.stringMatching(/^Internet Computer USD: \d/),
+      expect.stringMatching(new RegExp(`^${HELD_ICP_CARD_TITLE}: \\d`)),
+      expect.stringMatching(/^Internet Computer USD: \d/),
+      expect.stringMatching(/^Internet Computer ICP: \d/),
+      expect.stringMatching(new RegExp(`^${STAKED_ICP_CARD_TITLE}: \\d`)),
+    ]);
+});

frontend/src/tests/lib/components/portfolio/HeldTokensCard.spec.ts

@@ -130,6 +130,36 @@ describe("HeldTokensCard", () => {
     expect(nativeBalances).toEqual(["••• ICP", "••• ckBTC", "••• ckETH"]);
   });
 
+  it("should show the balances in the aria labels when privacy mode is off", async () => {
+    const po = renderComponent({
+      topHeldTokens: mockTokens,
+      usdAmount: 6000,
+    });
+
+    expect(await po.getHeldTokensBalanceInUsdAriaLabels()).toEqual([
+      "Internet Computer USD: 100",
+      "ckBTC USD: 200",
+      "ckETH USD: 300",
+    ]);
+    expect(await po.getAmountAriaLabel()).toBe("Tokens Balance: 6000");
+  });
+
+  it("should not expose the balances in the aria labels when privacy mode is on", async () => {
+    balancePrivacyOptionStore.set("hide");
+
+    const po = renderComponent({
+      topHeldTokens: mockTokens,
+      usdAmount: 6000,
+    });
+
+    expect(await po.getHeldTokensBalanceInUsdAriaLabels()).toEqual([
+      "Internet Computer USD: hidden",
+      "ckBTC USD: hidden",
+      "ckETH USD: hidden",
+    ]);
+    expect(await po.getAmountAriaLabel()).toBe("Tokens Balance: hidden");
+  });
+
   it("should render links for each row", async () => {
     const po = renderComponent({
       topHeldTokens: mockTokens,

frontend/src/tests/lib/components/portfolio/StakedTokensCard.spec.ts

@@ -177,6 +177,50 @@ describe("StakedTokensCard", () => {
     ]);
   });
 
+  it("should show the balances in the aria labels when privacy mode is off", async () => {
+    const po = renderComponent({
+      topStakedTokens: mockStakedTokens,
+      usdAmount: 5000,
+    });
+
+    expect(await po.getStakedTokensStakeInUsdAriaLabels()).toEqual([
+      "Internet Computer USD: 100",
+      "Project 1 USD: 200",
+      "Project 2 USD: 300",
+      "Project 3 USD: 400",
+    ]);
+    expect(await po.getStakedTokensStakeInNativeCurrencyAriaLabels()).toEqual([
+      "Internet Computer ICP: 0.01",
+      "Project 1 TET: 0.01",
+      "Project 2 TET: 0.01",
+      "Project 3 TET: 0.01",
+    ]);
+    expect(await po.getAmountAriaLabel()).toBe("Staking Balance: 5000");
+  });
+
+  it("should not expose the balances in the aria labels when privacy mode is on", async () => {
+    balancePrivacyOptionStore.set("hide");
+
+    const po = renderComponent({
+      topStakedTokens: mockStakedTokens,
+      usdAmount: 5000,
+    });
+
+    expect(await po.getStakedTokensStakeInUsdAriaLabels()).toEqual([
+      "Internet Computer USD: hidden",
+      "Project 1 USD: hidden",
+      "Project 2 USD: hidden",
+      "Project 3 USD: hidden",
+    ]);
+    expect(await po.getStakedTokensStakeInNativeCurrencyAriaLabels()).toEqual([
+      "Internet Computer ICP: hidden",
+      "Project 1 TET: hidden",
+      "Project 2 TET: hidden",
+      "Project 3 TET: hidden",
+    ]);
+    expect(await po.getAmountAriaLabel()).toBe("Staking Balance: hidden");
+  });
+
   it("should not show info row when numberOfTopHeldTokens is the same as the number of topStakedTokens", async () => {
     const po = renderComponent({
       topStakedTokens: mockStakedTokens.slice(0, 3),