Skip to content

Conversation

jonatack
Copy link
Member

@jonatack jonatack commented Sep 7, 2019

Closes #16820. The wallet name escaping issue in that issue predates #15450 and is fixed by #16826.

  • rename encrypt() to encryptWallet(), and blank() to makeBlankWallet() // EDIT: updated to
    isEncryptWalletChecked()
    isDisablePrivateKeysChecked()
    isMakeBlankWalletChecked()
  • fix naming of askPasshprase() to askPassphrase()
  • fix passphrase labels and tooltip in createwalletdialog.ui and askpassphrasedialog.ui
  • fix grammar of labels in askpassphrase dialog and WalletController::closeWallet
  • fix autofocus in CreateWalletActivity::askPassphrase()

Squashed down to three commits.

Reviewers, to test manually: build, launch the gui wallet, and look at labels/tooltips/focus with the create wallet, encrypt wallet, change password, and close wallet commands.

@jonatack jonatack mentioned this pull request Sep 7, 2019
@jonatack jonatack force-pushed the createwallet-followup branch from a24befd to 66af07e Compare September 8, 2019 09:21
@jonatack jonatack changed the title [WIP] gui: Createwallet followups gui: Create wallet menu option follow-ups Sep 8, 2019
@jonatack jonatack force-pushed the createwallet-followup branch from 66af07e to d4072b3 Compare September 8, 2019 10:52
@jonatack
Copy link
Member Author

jonatack commented Sep 8, 2019

Lone CI failure is a time-out.

@promag
Copy link
Contributor

promag commented Sep 8, 2019

Concept ACK, restarted travis job.

@jonatack jonatack force-pushed the createwallet-followup branch from d4072b3 to 52d3743 Compare September 8, 2019 12:44
@michaelfolkson
Copy link

michaelfolkson commented Sep 8, 2019

ACK 52d3743. Labels and tooltips correctly update.

@promag
Copy link
Contributor

promag commented Sep 8, 2019

Code review ACK 52d3743.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 8, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

as well as disablePrivateKeys() to be consistent in naming.
…edialog

UI improvements:
- update remaining GUI wallet labels and tooltips from passwords to passphrases
- improve grammar of labels in askpassphrase dialog and WalletController::closeWallet
@jonatack jonatack force-pushed the createwallet-followup branch from 52d3743 to 1310bff Compare September 9, 2019 08:59
@jonatack jonatack force-pushed the createwallet-followup branch from 1310bff to cad3ab5 Compare September 9, 2019 09:05
jonatack added a commit to jonatack/bitcoin-development that referenced this pull request Sep 9, 2019
@laanwj laanwj added this to the 0.19.0 milestone Sep 10, 2019
@kanzure
Copy link
Contributor

kanzure commented Sep 10, 2019

I am generally in favor of migrating from "password" to "passphrase". I made this comment in -wizards that nobody should talk about 'the password' in bip39 because 'word' already means a single word from the dictionary which is definitely low entropy. And then I was directed here.

@jonatack
Copy link
Member Author

Yes, removing the last user-facing references to "password" has been on my to-do list since June. Since PRs to change UI can invite bikeshedding (there's been a bit of that lately), I waited until related code or a PR (in this case, the PR this one follows up on) touched it to push the changes in 539d940.

@jb55
Copy link
Contributor

jb55 commented Sep 13, 2019

Approach ACK cad3ab5

@instagibbs
Copy link
Member

code review and tACK cad3ab5

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cad3ab5

Tested the auto focusing behaviour on macOS. Other changes seem ok.

to to for

fanquake added a commit to fanquake/bitcoin that referenced this pull request Sep 16, 2019
cad3ab5 gui: fix autofocus in CreateWalletActivity::askPassphrase() (Jon Atack)
539d940 gui: fix passphrase labels/tooltip in createwalletdialog/askpassphrasedialog (Jon Atack)
43aa9b0 gui: rename encrypt(), blank(), and askPasshprase() (Jon Atack)

Pull request description:

  Closes bitcoin#16820. The wallet [name escaping issue](bitcoin#15450 (review)) in that issue predates bitcoin#15450 and is fixed by bitcoin#16826.

  - [x]  rename encrypt() to encryptWallet(), and blank() to makeBlankWallet() // EDIT: updated to
          isEncryptWalletChecked()
          isDisablePrivateKeysChecked()
          isMakeBlankWalletChecked()
  - [x]  fix naming of askPasshprase() to askPassphrase()
  - [x]  fix passphrase labels and tooltip in createwalletdialog.ui and askpassphrasedialog.ui
  - [x]  fix grammar of labels in askpassphrase dialog and WalletController::closeWallet
  - [x]  fix autofocus in CreateWalletActivity::askPassphrase()

  Squashed down to three commits.

  Reviewers, to test manually: build, launch the gui wallet, and look at labels/tooltips/focus with the create wallet, encrypt wallet, change password, and close wallet commands.

ACKs for top commit:
  jb55:
    Approach ACK cad3ab5
  instagibbs:
    code review and tACK cad3ab5
  fanquake:
    ACK cad3ab5

Tree-SHA512: b441fbf8f8cd370dd692bac24f0d3c1b32fc7d947b6c3a2c9ba7cf0bc175a72b3460440f2f10f7632c0e8e0f8e65fe15615a30c46e2c7763bf258c504b457dd6
@fanquake fanquake merged commit cad3ab5 into bitcoin:master Sep 16, 2019
@jonatack jonatack deleted the createwallet-followup branch September 16, 2019 09:40
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 16, 2019
cad3ab5 gui: fix autofocus in CreateWalletActivity::askPassphrase() (Jon Atack)
539d940 gui: fix passphrase labels/tooltip in createwalletdialog/askpassphrasedialog (Jon Atack)
43aa9b0 gui: rename encrypt(), blank(), and askPasshprase() (Jon Atack)

Pull request description:

  Closes bitcoin#16820. The wallet [name escaping issue](bitcoin#15450 (review)) in that issue predates bitcoin#15450 and is fixed by bitcoin#16826.

  - [x]  rename encrypt() to encryptWallet(), and blank() to makeBlankWallet() // EDIT: updated to
          isEncryptWalletChecked()
          isDisablePrivateKeysChecked()
          isMakeBlankWalletChecked()
  - [x]  fix naming of askPasshprase() to askPassphrase()
  - [x]  fix passphrase labels and tooltip in createwalletdialog.ui and askpassphrasedialog.ui
  - [x]  fix grammar of labels in askpassphrase dialog and WalletController::closeWallet
  - [x]  fix autofocus in CreateWalletActivity::askPassphrase()

  Squashed down to three commits.

  Reviewers, to test manually: build, launch the gui wallet, and look at labels/tooltips/focus with the create wallet, encrypt wallet, change password, and close wallet commands.

ACKs for top commit:
  jb55:
    Approach ACK cad3ab5
  instagibbs:
    code review and tACK cad3ab5
  fanquake:
    ACK cad3ab5

Tree-SHA512: b441fbf8f8cd370dd692bac24f0d3c1b32fc7d947b6c3a2c9ba7cf0bc175a72b3460440f2f10f7632c0e8e0f8e65fe15615a30c46e2c7763bf258c504b457dd6
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 20, 2020
…ys()

Summary:
to be consistent in naming.

Backport of Core [[bitcoin/bitcoin#16822 | PR16822]] - part 1 of 3
Commit bitcoin/bitcoin@43aa9b0

Note: on our side the spelling of the method name `askPassphrase` was already good
when it was first backported (see D7105)

Test Plan:
Method renaming without change in the logic.
Just compile to make sure it didn't break anything.
`ninja`

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Subscribers: jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8006
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 20, 2020
…etdialog/askpassphrasedialog

Summary:
UI improvements:
- update remaining GUI wallet labels and tooltips from passwords to passphrases
- improve grammar of labels in askpassphrase dialog and WalletController::closeWallet

Backport of Core [[bitcoin/bitcoin#16822 | PR16822]] - part 2 of 3
Commit bitcoin/bitcoin@539d940

Depends on D8006

Test Plan:
`ninja`

Run `src/qt/bitcoin-qt`, create a new wallet with encryption and check the text in the dialogs.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8007
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 21, 2020
…assphrase()

Summary:
Backport of Core [[bitcoin/bitcoin#16822 | PR16822]] - part 3 of 3
Commit bitcoin/bitcoin@cad3ab5

Depends on D8007

Test Plan: Run `src/qt/bitcoin-qt`, create a new wallet with encryption and check that the dialog is modal.

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8008
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Follow ups from #15450
9 participants