-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Address security concern dapps #13398
Conversation
...org/chromium/chrome/browser/crypto_wallet/fragments/dapps/AddSwitchChainNetworkFragment.java
Outdated
Show resolved
Hide resolved
...org/chromium/chrome/browser/crypto_wallet/fragments/dapps/AddSwitchChainNetworkFragment.java
Outdated
Show resolved
Hide resolved
Try to use domain layer as much as possible to avoid future refactoring. |
While you are on signature panel, Could you also please add some bottom padding to the description and close the brave/brave-browser#22913 ticket, TIA. |
I'll start using domain layer from my next ticket, I just wanted to develop little more understanding around it. |
++ but please remove unused |
iOS CI failed because of a known problem not related to that PR |
Verification passed on Oppo Reno 5 with Android 12 running 1.41.18 custom Dapp support build
|
A DApp is requesting your public encryption key | ||
</message> | ||
<message name="IDS_BRAVE_WALLET_PROVIDE_ENCRYPTION_KEY_DESCRIPTION" desc="Provide encryption key panel details"> | ||
is requesting your wallets public encryption key. If you consent to providing this key, the site will be able to compose encrypted messages to you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wallet's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes @qamarngr could you pls fix and uplift?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same in IDS_BRAVE_WALLET_PROVIDE_ENCRYPTION_KEY_TITLE
in components/resources/wallet_strings.grdp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also looks like this string isn't using a proper variable for the origin in question. Languages can't be translated 1:1 via concatenation like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bbondy will fix it this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkarolin What should be fixed in IDS_BRAVE_WALLET_PROVIDE_ENCRYPTION_KEY_TITLE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves brave/brave-browser#21798
Resolves brave/brave-browser#22913
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Same as in PR