sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

fix: "Wrong password." is a sentence fragment and diverges from the other password prompts #172

New Issue
Closed
opened 2026-08-09 04:53:47 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-09 04:53:47 +02:00
Collaborator
Copy Link

Problem

src/popup/views/confirmTx.js renders "Wrong password." on a failed
decryption. That is a fragment, not a full sentence, and both README.md
Language & Labeling ("Error messages are full sentences") and RULES.md:120
require full sentences.

It is also now inconsistent. PR #171 (issue #157) moved decryption into the
popup for the two dApp approval paths and introduced the wording
"That password is incorrect. Please try again." there. So after #171 lands,
the wallet has two different messages for the identical condition, on screens a
user can reach minutes apart.

Found during the #157 work and deliberately left out of scope there to keep
that security PR reviewable.

Implementation requirements

  • Make all password-failure messages identical across every screen that has
    one. Audit for the full set before editing — at minimum confirmTx.js, the
    two approval paths touched by #171, plus the delete-wallet confirm
    (src/popup/views/deleteWallet.js:52) and the private key export
    (src/popup/views/addressDetail.js:338), which are the other two
    decryptWithPassword call sites.
  • Use the wording #171 introduced, "That password is incorrect. Please try again.", unless there is a reason to prefer another — in which case change
    them all, including #171's, and say why.
  • Do not change any behaviour, only the strings. A wrong password must still
    fail closed exactly as it does today.
  • Watch for layout shift: per the README No Layout Shift policy, error areas
    reserve their space. A longer string must not reflow the surrounding UI.
    Check each screen's error container can hold the new text at the 360px popup
    width without moving anything.
  • If #171 has not landed when you start, coordinate: do not create a conflict
    in the approval paths. Prefer rebasing onto it.

Definition of done

  • Every password-failure message in the popup is the same full sentence.
  • All decryptWithPassword call sites have been audited, and the PR lists
    which screens were checked.
  • No behaviour change; wrong passwords still fail closed and do not resolve
    any pending approval.
  • No layout shift on any affected screen at 360px width.
  • TODO.md updated in the same commit.
  • make check passes.
## Problem `src/popup/views/confirmTx.js` renders `"Wrong password."` on a failed decryption. That is a fragment, not a full sentence, and both `README.md` Language & Labeling ("Error messages are full sentences") and `RULES.md:120` require full sentences. It is also now inconsistent. PR #171 (issue #157) moved decryption into the popup for the two dApp approval paths and introduced the wording `"That password is incorrect. Please try again."` there. So after #171 lands, the wallet has two different messages for the identical condition, on screens a user can reach minutes apart. Found during the #157 work and deliberately left out of scope there to keep that security PR reviewable. ## Implementation requirements - Make all password-failure messages identical across every screen that has one. Audit for the full set before editing — at minimum `confirmTx.js`, the two approval paths touched by #171, plus the delete-wallet confirm (`src/popup/views/deleteWallet.js:52`) and the private key export (`src/popup/views/addressDetail.js:338`), which are the other two `decryptWithPassword` call sites. - Use the wording #171 introduced, `"That password is incorrect. Please try again."`, unless there is a reason to prefer another — in which case change them all, including #171's, and say why. - Do not change any behaviour, only the strings. A wrong password must still fail closed exactly as it does today. - Watch for layout shift: per the README No Layout Shift policy, error areas reserve their space. A longer string must not reflow the surrounding UI. Check each screen's error container can hold the new text at the 360px popup width without moving anything. - If #171 has not landed when you start, coordinate: do not create a conflict in the approval paths. Prefer rebasing onto it. ## Definition of done - [ ] Every password-failure message in the popup is the same full sentence. - [ ] All `decryptWithPassword` call sites have been audited, and the PR lists which screens were checked. - [ ] No behaviour change; wrong passwords still fail closed and do not resolve any pending approval. - [ ] No layout shift on any affected screen at 360px width. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.
clawbot added this to the 1.0.0 milestone 2026-08-09 04:53:47 +02:00
clawbot commented 2026-08-12 11:32:14 +02:00
Author
Collaborator
Copy Link

Done in #264.

There were three wordings, not two. confirmTx.js and deleteWallet.js said "Wrong password."; showPhrase.js and exportPrivkey.js said "That password is not correct. Please try again."; the two approval paths said "That password is incorrect. Please try again." All five files now say the last one.

Two corrections to the issue's list: the private key export moved out of addressDetail.js into its own src/popup/views/exportPrivkey.js, and showPhrase.js was not listed but is a decryptWithPassword call site. Call sites were found by scanning src/, not from the list.

Verified: make check green after rebasing onto next at c6a1f97 (26 suites, 588 tests, test-verify-build 18 cases, prettier clean); make test-e2e 17/17 in the pinned container. Layout measured in that same container at a 360x600 viewport against the built extension — all four changed containers stay at 20px with the message shown, the next element in flow moves 0px, and the text renders on one line at 232.23px in a 368px box. Per-screen numbers are in the PR body.

Diff is six string literals and one test expectation: no behaviour change, and nothing in the approval interlock touched.

Done in [#264](https://git.eeqj.de/sneak/AutistMask/pulls/264). There were three wordings, not two. `confirmTx.js` and `deleteWallet.js` said `"Wrong password."`; `showPhrase.js` and `exportPrivkey.js` said `"That password is not correct. Please try again."`; the two approval paths said `"That password is incorrect. Please try again."` All five files now say the last one. Two corrections to the issue's list: the private key export moved out of `addressDetail.js` into its own `src/popup/views/exportPrivkey.js`, and `showPhrase.js` was not listed but is a `decryptWithPassword` call site. Call sites were found by scanning `src/`, not from the list. Verified: `make check` green after rebasing onto `next` at `c6a1f97` (26 suites, 588 tests, `test-verify-build` 18 cases, prettier clean); `make test-e2e` 17/17 in the pinned container. Layout measured in that same container at a 360x600 viewport against the built extension — all four changed containers stay at 20px with the message shown, the next element in flow moves 0px, and the text renders on one line at 232.23px in a 368px box. Per-screen numbers are in the PR body. Diff is six string literals and one test expectation: no behaviour change, and nothing in the approval interlock touched.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#172
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?