fix: open no approval window for a site-connection prompt already answered (closes #287)

When a site-connection prompt was decided before the toolbar popup raised
for it had loaded, that popup was torn down, chrome.action.openPopup()
rejected, and the background opened its fallback window for the answered
approval and only then removed it. In the Chrome end-to-end suite the next
test could take that window for its own prompt and lose it under its wait.
openApprovalWindow() now returns before creating a window when the approval
is no longer pending.

The blocklist test clicked its self-closing Reject with a plain click; it
now clicks it as the other site Reject does, with the click witnessed.

Model: opus-5-5

README.md

@@ -619,15 +619,13 @@ The jobs **report, they do not gate.** A failure is a red mark against the
 commit that a reviewer has to account for, not a hard block: whether a check
 blocks a merge is Gitea branch protection, which this repo does not configure.
 
-That is not only a statement about configuration. Reports of the Chrome suite
+That is not only a statement about configuration. One report of the Chrome suite
-**failing under load** are still open, among them
+**failing under load** is still open:
-[#290](https://git.eeqj.de/sneak/AutistMask/issues/290), runs on a busy machine
+[#290](https://git.eeqj.de/sneak/AutistMask/issues/290) records runs on a busy
-failing with `the extension opened no approval window within 30000ms`, and
+machine failing with `the extension opened no approval window within 30000ms`.
-[#446](https://git.eeqj.de/sneak/AutistMask/issues/446), the Settings round trip
+So a red `e2e-chrome` has to be read before it is believed, and that report is
-closing the popup before its network switch is saved. So a red `e2e-chrome` has
+the blocker to ever making this a required check. Do not answer it with a retry
-to be read before it is believed, and those failures are the blocker to ever
+wrapper: a suite that reruns until it is green stops being evidence.
-making this a required check. Do not answer them with a retry wrapper: a suite
-that reruns until it is green stops being evidence.
 
 Nothing in either job can pass vacuously. There is no `continue-on-error` and no
 `|| true`; both scripts exit non-zero when docker is missing, when the image
@@ -696,7 +694,6 @@ src/
     balances.js       — ETH + ERC-20 balance fetching via RPC + Blockscout
     constants.js      — chain IDs, default RPC endpoint, ERC-20 ABI
     ens.js            — ENS forward/reverse resolution (popup only)
-    holders.js        — holder-count parsing and the low-holder rule
     prices.js         — ETH/USD and token/USD via CoinDesk API
     scamlist.js       — known fraud contract addresses
     state.js          — persisted state (extension storage)
@@ -1125,18 +1122,13 @@ claiming a symbol that belongs to the native asset and therefore has no
 legitimate contract at all (`"ETH"`, and every network's `nativeCurrency`, such
 as `"SepoliaETH"`, on every network). That filter is unconditional — the "Hide
 tokens with fewer than 1,000 holders" setting governs the transaction history
-and the send-screen token selector, not this list. A token's holder count is
+and the send-screen token selector, not this list. `fetchTokenBalances()` stores
-unknown when the explorer reports none, or reports anything other than a whole
+every nonzero holding of a token it admits, however small, but a holding below
-number written in digits alone, such as `1,000` or `1e3` (`parseHoldersCount()`
+0.000001 is left out of the balance lists, the send-screen token selector, the
-in `src/shared/holders.js`). This list does not take an unknown count as 1,000
+address total and the remove-address warning (`isBelowOneMillionth()` in
-or more, so such a token is shown only when it is on the bundled list or
+`src/shared/amountDisplay.js`). The Send and confirmation screens show it when
-tracked. `fetchTokenBalances()` stores every nonzero holding of a token it
+its token is the one being sent. Tracked tokens with a zero balance are listed
-admits, however small, but a holding below 0.000001 is left out of the balance
+as well while "Show tracked tokens with zero balance" is on.
-lists, the send-screen token selector, the address total and the remove-address
-warning (`isBelowOneMillionth()` in `src/shared/amountDisplay.js`). The Send and
-confirmation screens show it when its token is the one being sent. Tracked
-tokens with a zero balance are listed as well while "Show tracked tokens with
-zero balance" is on.
 
 #### Stored state and its version
 
@@ -1445,9 +1437,7 @@ view would leave a wallet one click from deletion.
     - Send / Receive buttons
     - Token contract well (ERC-20 only): full contract address (tap to copy,
       etherscan link) plus name, symbol, decimals, holder count and project
-      website where known. The "Holders:" row is left out, not shown as 0, when
+      website where known
-      the token's balance-list entry has no holder count: the explorer did not
-      report a readable one, or the token is not in the balance list
     - Token-filtered transaction list (only this token's transfers)
 - **Transitions**:
     - "Send" → **Send** (token locked: the dropdown is replaced by a static
@@ -2398,8 +2388,7 @@ indexes it as a real token transfer.
   fewer than 1,000 holders are hidden from transaction history by default.
   Legitimate tokens have substantial holder counts; poisoning tokens typically
   have zero. This catches new poisoning contracts that use novel symbols not in
-  the known token list. A transfer whose token's holder count is unknown (see
+  the known token list.
-  Data Model) is kept: only a reported count below 1,000 hides it.
 
 - **Fraud contract blocklist**: AutistMask maintains a local list of known fraud
   contract addresses. Token transfers involving these contracts are filtered
@@ -2409,9 +2398,7 @@ indexes it as a real token transfer.
 - **Send-side token filtering**: Tokens with fewer than 1,000 holders are
   excluded from the token selector on the send screen. This prevents users from
   accidentally interacting with a spoofed token that appeared in their balance
-  via a fake Transfer event. A token whose holder count is unknown is kept in
+  via a fake Transfer event.
-  the selector. The selector offers only tokens in the balance list, so such a
-  token is one on the bundled list or one the user tracks.
 
 - **Dust transaction filtering**: A second wave of the same attack used real
   native ETH transfers instead of fake tokens. Transaction
@@ -2435,9 +2422,8 @@ indexes it as a real token transfer.
   both cases identically to the history. The fraud contract blocklist is applied
   unconditionally on that selector and is not consulted by the balance list at
   all. The low-holder setting also gates the send selector, while the balance
-  list's own 1,000-holder floor is unconditional (see Data Model). An unknown
+  list's own 1,000-holder floor is unconditional (see Data Model). The dust
-  holder count passes the history and send-selector filters but not that floor.
+  threshold applies to the transaction history alone.
-  The dust threshold applies to the transaction history alone.
 
 #### Phishing Domain Protection
 

TODO.md

@@ -56,17 +56,6 @@ but the review is broader than any of them.
   approval that is no longer pending. The blocklist test's Reject, whose window
   closes itself, is clicked as the other site Reject is, with the click
   witnessed.
-- 2026-10-05: A `holders_count` that is not a whole number in plain digits is
-  unknown, not read in part
-  ([#251](https://git.eeqj.de/sneak/AutistMask/issues/251)). `parseInt` read
-  `1,000` as 1, `0x10` as 0 and `1e3` as 1, a reported low count that hides the
-  token in the transaction history and the send-screen token selector. A count
-  above `Number.MAX_SAFE_INTEGER` is unknown too, not rounded or `Infinity`. The
-  balance list's `holders !== null` check, which did nothing, is dropped.
-  `README.md` and `docs/README.md` now say how each filter treats an unknown
-  count and that the token screen leaves out its "Holders:" row then, and
-  `README.md` lists `src/shared/holders.js`.
-
 - 2026-10-04: A popup boot in the tests loads transactions without failing
   ([#429](https://git.eeqj.de/sneak/AutistMask/issues/429)). The stand-in for
   `filterTransactions` in `tests/support/popupBoot.js` returned a bare list,

docs/README.md

@@ -333,13 +333,7 @@ it is hidden from your transaction history and from the send token list.
 from transaction history and the send token list, and are left out of your
 balances unless they are on the bundled known-token list or you added them
 yourself. Legitimate tokens have substantial holder counts; scam tokens deployed
-for address poisoning typically have zero. When the explorer reports no holder
+for address poisoning typically have zero.
-count for a token, or reports something other than a whole number in plain
-digits (such as "1,000"), the count is unknown. An unknown count does not hide a
-token from your transaction history or the send token list, and it does not get
-a token into your balances either: such a token is listed only if it is on the
-bundled known-token list or you added it yourself. The screen you reach by
-clicking a token balance shows a "Holders:" line only when the count is known.
 
 **Fraud contract blocklist.** When AutistMask detects a fraudulent transfer, it
 adds the contract address to a local blocklist. Future transactions from that

src/shared/balances.js

@@ -147,20 +147,20 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
             // null is a holding of an amount that cannot be stated, which is
             // not the same as a holding of zero, and must never render as one.
             const bal = scale === null ? null : formatTokenBalance(raw, scale);
-            // null means the explorer reported no readable count, which is
+            // null means the explorer reported no count, which is not the
-            // not the same as a count of zero.  This gate is not the
+            // same as a count of zero.  This gate is not the low-holder
-            // low-holder display filter: it has no user-facing off switch and
+            // display filter: it has no user-facing off switch and governs
-            // governs the whole balance list, so it stays strict and admits a
+            // the whole balance list, so it stays strict and admits a token
-            // token only on a reported count — an unreported one is no
+            // only on a reported count — an unreported one is no evidence.
-            // evidence, and `null >= LOW_HOLDER_THRESHOLD` is false.  A
+            // A legitimate token still reaches the list through the known
-            // legitimate token still reaches the list through the known
             // token list or by the user tracking it, and the null is carried
             // through to the views, where the two low-holder filters treat
             // an unknown count as "do not judge" rather than as zero.
             const holders = parseHoldersCount(item.token.holders_count);
             const isKnown = TOKEN_BY_ADDRESS.has(tokenAddr);
             const isTracked = trackedSet.has(tokenAddr);
-            const hasEnoughHolders = holders >= LOW_HOLDER_THRESHOLD;
+            const hasEnoughHolders =
+                holders !== null && holders >= LOW_HOLDER_THRESHOLD;
 
             // Skip spam tokens the user never asked to see
             if (!isKnown && !isTracked && !hasEnoughHolders) continue;

src/shared/holders.js

@@ -9,22 +9,13 @@
 
 const LOW_HOLDER_THRESHOLD = 1000;
 
-// Parse an explorer-supplied holders_count into a number, or null when it is
+// Parse an explorer-supplied holders_count into a number, or null when the
-// not one.  Only a whole number of zero or more, or a string made of nothing
+// explorer did not report one.  Anything unparseable is unknown too: a count
-// but the digits 0-9, is a count.  Anything else is null, never read in part:
+// we cannot read is not a count of zero.
-// "1,000", "0x10" and "1e3" are unknown, not 1, 0 and 1, because a count we
-// cannot read is not a low count.  A count above Number.MAX_SAFE_INTEGER is
-// null too: a number cannot hold it exactly, so it would come back rounded,
-// or as Infinity.
 function parseHoldersCount(raw) {
-    if (typeof raw === "number") {
+    if (raw === null || raw === undefined || raw === "") return null;
-        return Number.isSafeInteger(raw) && raw >= 0 ? raw : null;
+    const n = parseInt(raw, 10);
-    }
+    return Number.isFinite(n) ? n : null;
-    if (typeof raw === "string" && /^[0-9]+$/.test(raw)) {
-        const count = Number(raw);
-        return Number.isSafeInteger(count) ? count : null;
-    }
-    return null;
 }
 
 // True only for a token the explorer reported as having fewer holders than

src/shared/transactions.js

@@ -137,10 +137,9 @@ function parseTokenTransfer(tt, addrLower, chainId) {
         contractAddress: normalizeAddress(
             tt.token?.address_hash || tt.token?.address || "",
         ),
-        // null when the explorer reported no readable count: unknown, not
+        // null when the explorer reported no count: unknown, not zero.  The
-        // zero.  The low-holder filter declines to judge a null, so a
+        // low-holder filter declines to judge a null, so a legitimate token
-        // legitimate token is not hidden because a field went missing
+        // is not hidden because a field went missing upstream.
-        // upstream.
         holders: parseHoldersCount(tt.token?.holders_count),
         chainId: chainId,
     };

tests/holders.test.js

@@ -57,39 +57,6 @@ describe("parseHoldersCount", () => {
         expect(parseHoldersCount("many")).toBeNull();
         expect(parseHoldersCount(NaN)).toBeNull();
     });
-
-    // Each of these starts with a digit, so reading only the leading digits
-    // would turn it into a small reported count, and a small count is
-    // exactly what hides a token as spam (issue #251).
-    test.each(["1,000", "0x10", "1e3", "12 holders"])(
-        "%p is not read in part: it is unknown",
-        (raw) => {
-            expect(parseHoldersCount(raw)).toBeNull();
-        },
-    );
-
-    test("a negative count is unknown", () => {
-        expect(parseHoldersCount("-5")).toBeNull();
-        expect(parseHoldersCount(-5)).toBeNull();
-    });
-
-    // A number holds a whole number exactly only up to 2^53 - 1.  Past that a
-    // string of digits would come back rounded, and a long enough one as
-    // Infinity, which would pass every holder-count floor.
-    test("a count too large for a number to hold exactly is unknown", () => {
-        expect(parseHoldersCount("9007199254740993")).toBeNull();
-        expect(parseHoldersCount("9".repeat(400))).toBeNull();
-        expect(parseHoldersCount(2 ** 53)).toBeNull();
-    });
-
-    test("the largest count a number holds exactly still parses", () => {
-        expect(parseHoldersCount("9007199254740991")).toBe(
-            Number.MAX_SAFE_INTEGER,
-        );
-        expect(parseHoldersCount(Number.MAX_SAFE_INTEGER)).toBe(
-            Number.MAX_SAFE_INTEGER,
-        );
-    });
 });
 
 describe("isLowHolderCount", () => {