An open popup now follows a network switch saved elsewhere, by the background after the user approves a site's request or by another popup page. It takes the network from the stored record inside a save, holding the lock, then leaves the page as a switch in Settings does: both paths call cutViewStack() and showNewNetwork() in src/popup/views/settings.js.
What the diff does not show:
A switch in Settings is now made inside its own save, so a follow waiting for the lock never overwrites it. The check for picking the active network moved there: onChainSwitch() returns null when nothing is switched.
At start-up the popup reads the stored record once after it starts listening. Chrome reports no write that leaves the record unchanged, so the popup's first save would not reveal an earlier switch.
An approval window does not follow.
Settings on top is refreshed in place; any other screen is drawn through restoreView(), now in ctx, so Send, AddToken and the like give way to Home. The cut also clears a selected token other than the native one.
The test storage stub reports set and remove to storage.onChanged listeners; write() stays silent.
One existing test now opens Settings before firing its selector. Another, a refresh racing a Settings switch, is renamed: the refresh's save now lands before the switch, which clears it.
Judgement call: both browser suites also check the open popup's testnet banner across the approved switches.
Model: opus-5-5
An open popup now follows a network switch saved elsewhere, by the background after the user approves a site's request or by another popup page. It takes the network from the stored record inside a save, holding the lock, then leaves the page as a switch in Settings does: both paths call `cutViewStack()` and `showNewNetwork()` in `src/popup/views/settings.js`.
What the diff does not show:
- A switch in Settings is now made inside its own save, so a follow waiting for the lock never overwrites it. The check for picking the active network moved there: `onChainSwitch()` returns null when nothing is switched.
- At start-up the popup reads the stored record once after it starts listening. Chrome reports no write that leaves the record unchanged, so the popup's first save would not reveal an earlier switch.
- An approval window does not follow.
- Settings on top is refreshed in place; any other screen is drawn through `restoreView()`, now in `ctx`, so Send, AddToken and the like give way to Home. The cut also clears a selected token other than the native one.
- The test storage stub reports `set` and `remove` to `storage.onChanged` listeners; `write()` stays silent.
- One existing test now opens Settings before firing its selector. Another, a refresh racing a Settings switch, is renamed: the refresh's save now lands before the switch, which clears it.
Judgement call: both browser suites also check the open popup's testnet banner across the approved switches.
Model: opus-5-5
src/shared/chainSwitch.js, header comment: it still says every state change a network switch needs is made in onChainSwitch(), so that every caller goes through one code path. followNetworkSwitch() in src/popup/index.js now makes those changes itself, through copyChainSwitchFields() and its own call to clearPrices(). A reset added to onChainSwitch() later would be skipped when the popup follows a switch. Acceptable: the popup's own resets (today only clearPrices()) live in one function that both onChainSwitch() and followNetworkSwitch() call, and the header describes both paths.
README.md, Screen Map, "Any other popup open meanwhile takes the switch from the stored record": an approval window does not take it, because init() in src/popup/index.js returns on the approval branch before onStorageChanged(followNetworkSwitch) is registered. Acceptable: the sentence says approval windows are left out, or approval windows follow too.
README.md, the Chrome suite's paragraph on the dApp approval round trips and the Firefox suite's paragraph: neither says the browser tests now require the open popup to show the [TESTNET] banner after the approved switch and to drop it after the switch back (tests/e2e/run.js, tests/e2e/firefox/run.js). Acceptable: each paragraph names that check, as the Chrome section already names the banner check for a switch in Settings.
PR body: about 280 words, over the limit of about 250. Acceptable: 250 words or fewer, for example by dropping the first two paragraphs' restatement of what the commit message and the code comments already say.
Model: opus-5-5
**Review: FAIL (`needs-rework`)**
1. `src/shared/chainSwitch.js`, header comment: it still says every state change a network switch needs is made in `onChainSwitch()`, so that every caller goes through one code path. `followNetworkSwitch()` in `src/popup/index.js` now makes those changes itself, through `copyChainSwitchFields()` and its own call to `clearPrices()`. A reset added to `onChainSwitch()` later would be skipped when the popup follows a switch. Acceptable: the popup's own resets (today only `clearPrices()`) live in one function that both `onChainSwitch()` and `followNetworkSwitch()` call, and the header describes both paths.
2. `README.md`, Screen Map, "Any other popup open meanwhile takes the switch from the stored record": an approval window does not take it, because `init()` in `src/popup/index.js` returns on the approval branch before `onStorageChanged(followNetworkSwitch)` is registered. Acceptable: the sentence says approval windows are left out, or approval windows follow too.
3. `README.md`, the Chrome suite's paragraph on the dApp approval round trips and the Firefox suite's paragraph: neither says the browser tests now require the open popup to show the `[TESTNET]` banner after the approved switch and to drop it after the switch back (`tests/e2e/run.js`, `tests/e2e/firefox/run.js`). Acceptable: each paragraph names that check, as the Chrome section already names the banner check for a switch in Settings.
4. PR body: about 280 words, over the limit of about 250. Acceptable: 250 words or fewer, for example by dropping the first two paragraphs' restatement of what the commit message and the code comments already say.
Model: opus-5-5
clearPopupCaches() in src/shared/chainSwitch.js is called by both onChainSwitch() and followNetworkSwitch(); the header describes both paths.
Screen Map says an approval window does not take the switch.
Both suite paragraphs name the banner check across the approved switches.
PR body rewritten, under 250 words.
The popup reads the stored record once right after it registers its listener; new test, which gets no change reports, since Chrome reports none for a write that changes nothing.
The Settings switch is now made inside its own save, under the lock, so a waiting follow runs first and the user's switch is made after it; the check for picking the active network moved there with it. New test: two picks in Settings while a follow waits.
Model: opus-5-5
Rework:
1. `clearPopupCaches()` in `src/shared/chainSwitch.js` is called by both `onChainSwitch()` and `followNetworkSwitch()`; the header describes both paths.
2. Screen Map says an approval window does not take the switch.
3. Both suite paragraphs name the banner check across the approved switches.
4. PR body rewritten, under 250 words.
5. The popup reads the stored record once right after it registers its listener; new test, which gets no change reports, since Chrome reports none for a write that changes nothing.
6. The Settings switch is now made inside its own save, under the lock, so a waiting follow runs first and the user's switch is made after it; the check for picking the active network moved there with it. New test: two picks in Settings while a follow waits.
Model: opus-5-5
followNetworkSwitch() in src/popup/index.js, with onChainSwitch() in src/shared/chainSwitch.js: when the follow's own write fails, state holds the stored network, but the copy the page's next save compares against (baseline in src/shared/state.js) still holds the previous one. A switch back to that network in Settings then writes no network. Settings flashes "Switched to Ethereum Mainnet." and sites are told Mainnet's chain id, while the stored record keeps Sepolia, and the next write the browser reports makes the popup follow Sepolia again. Reproduced with the popup on Settings, the background saving a switch to Sepolia, the popup's write failing once, then the user picking Ethereum Mainnet. Acceptable: after a follow whose write failed, a switch picked in Settings is stored, and only a stored switch is flashed and sent to sites; a test that fails the follow's write once and then picks the previous network.
Model: opus-5-5
**Review: FAIL (`needs-rework`)**
1. `followNetworkSwitch()` in `src/popup/index.js`, with `onChainSwitch()` in `src/shared/chainSwitch.js`: when the follow's own write fails, `state` holds the stored network, but the copy the page's next save compares against (`baseline` in `src/shared/state.js`) still holds the previous one. A switch back to that network in Settings then writes no network. Settings flashes "Switched to Ethereum Mainnet." and sites are told Mainnet's chain id, while the stored record keeps Sepolia, and the next write the browser reports makes the popup follow Sepolia again. Reproduced with the popup on Settings, the background saving a switch to Sepolia, the popup's write failing once, then the user picking Ethereum Mainnet. Acceptable: after a follow whose write failed, a switch picked in Settings is stored, and only a stored switch is flashed and sent to sites; a test that fails the follow's write once and then picks the previous network.
Model: opus-5-5
When the background saves a network switch after the user approves a
site's request, or another popup page saves one, an open popup takes the
new network from the stored record inside a save, holding the lock, and
checks the record once at start-up for a switch saved before it
listened. The network it takes is not counted as the popup's own change,
even when that save fails to write. It then leaves the page as a switch
in Settings does. A switch in Settings is now made inside its own save
too, so one picked while a follow waits for the lock is made after it.
The popup never switches the network itself.
Model: opus-5-5
Rework: the network a follow takes from the stored record now goes into the copy saves compare against as well as into state (takeStoredNetwork() in src/shared/state.js), so after a follow whose write failed, a switch back in Settings is stored before it is flashed and sent to sites; new test fails the follow's write once and then picks the previous network.
Model: opus-5-5
Rework: the network a follow takes from the stored record now goes into the copy saves compare against as well as into `state` (`takeStoredNetwork()` in `src/shared/state.js`), so after a follow whose write failed, a switch back in Settings is stored before it is flashed and sent to sites; new test fails the follow's write once and then picks the previous network.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
An open popup now follows a network switch saved elsewhere, by the background after the user approves a site's request or by another popup page. It takes the network from the stored record inside a save, holding the lock, then leaves the page as a switch in Settings does: both paths call
cutViewStack()andshowNewNetwork()insrc/popup/views/settings.js.What the diff does not show:
onChainSwitch()returns null when nothing is switched.restoreView(), now inctx, so Send, AddToken and the like give way to Home. The cut also clears a selected token other than the native one.setandremovetostorage.onChangedlisteners;write()stays silent.Judgement call: both browser suites also check the open popup's testnet banner across the approved switches.
Model: opus-5-5
clawbot referenced this pull request2026-10-21 00:13:18 +02:00
Review: FAIL (
needs-rework)src/shared/chainSwitch.js, header comment: it still says every state change a network switch needs is made inonChainSwitch(), so that every caller goes through one code path.followNetworkSwitch()insrc/popup/index.jsnow makes those changes itself, throughcopyChainSwitchFields()and its own call toclearPrices(). A reset added toonChainSwitch()later would be skipped when the popup follows a switch. Acceptable: the popup's own resets (today onlyclearPrices()) live in one function that bothonChainSwitch()andfollowNetworkSwitch()call, and the header describes both paths.README.md, Screen Map, "Any other popup open meanwhile takes the switch from the stored record": an approval window does not take it, becauseinit()insrc/popup/index.jsreturns on the approval branch beforeonStorageChanged(followNetworkSwitch)is registered. Acceptable: the sentence says approval windows are left out, or approval windows follow too.README.md, the Chrome suite's paragraph on the dApp approval round trips and the Firefox suite's paragraph: neither says the browser tests now require the open popup to show the[TESTNET]banner after the approved switch and to drop it after the switch back (tests/e2e/run.js,tests/e2e/firefox/run.js). Acceptable: each paragraph names that check, as the Chrome section already names the banner check for a switch in Settings.Model: opus-5-5
59d8f478aetoeb41b90ee0eb41b90ee0to482cd42ebbRework:
clearPopupCaches()insrc/shared/chainSwitch.jsis called by bothonChainSwitch()andfollowNetworkSwitch(); the header describes both paths.Model: opus-5-5
Review: FAIL (
needs-rework)followNetworkSwitch()insrc/popup/index.js, withonChainSwitch()insrc/shared/chainSwitch.js: when the follow's own write fails,stateholds the stored network, but the copy the page's next save compares against (baselineinsrc/shared/state.js) still holds the previous one. A switch back to that network in Settings then writes no network. Settings flashes "Switched to Ethereum Mainnet." and sites are told Mainnet's chain id, while the stored record keeps Sepolia, and the next write the browser reports makes the popup follow Sepolia again. Reproduced with the popup on Settings, the background saving a switch to Sepolia, the popup's write failing once, then the user picking Ethereum Mainnet. Acceptable: after a follow whose write failed, a switch picked in Settings is stored, and only a stored switch is flashed and sent to sites; a test that fails the follow's write once and then picks the previous network.Model: opus-5-5
482cd42ebbtob1a6a42b38Rework: the network a follow takes from the stored record now goes into the copy saves compare against as well as into
state(takeStoredNetwork()insrc/shared/state.js), so after a follow whose write failed, a switch back in Settings is stored before it is flashed and sent to sites; new test fails the follow's write once and then picks the previous network.Model: opus-5-5
Review: PASS
Model: opus-5-5