-
Notifications
You must be signed in to change notification settings - Fork 869
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
Reassign Ctrl/Cmd + C default hotkey to copy clean urls #16764
Conversation
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.
lgtm.
1fa541c
to
1c1cca6
Compare
Is there a way to hide "copy clean link" entirely when the selection is just text and not a URL? |
1c1cca6
to
8135599
Compare
])"); | ||
const std::string test_url( | ||
"https://dev-pages.bravesoftware.com/clean-urls/" | ||
"exempted/" |
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's a little weird to be using exempted/
in these test cases because the JSON config we supply here does not in fact exempt this URL.
I'd suggest removing /exempted/
in these three new test cases to reduce potential confusion.
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.
done
|
1 similar comment
|
discussed in Slack, it is hidden when simple text selected, updated screenshots in the description |
8e8935b
to
15c7cc5
Compare
|
15c7cc5
to
1286696
Compare
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.
++
bool is_url = const_cast<BraveOmniboxViewViews*>(this)->SelectedTextIsURL(); | ||
if (is_url) { |
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.
bool is_url = const_cast<BraveOmniboxViewViews*>(this)->SelectedTextIsURL(); | |
if (is_url) { | |
if (SelectedTextIsURL()) { |
I don't understand why this feature was added to the latest version? Why would we want to copy the clean link when using the basic CMD + C shortcut for copying text off a page? You can see here the community considers it a bug? Is there a way for us to revert this behavior back to the default behavior we are all used to of CMD+C copying text? https://community.brave.com/t/keyboard-shortcuts-cmd-c-copy-clean-link-issue/477306 |
I agree with this suggestion? why would we want to copy the clean link when we are highlighting text and clearly trying to copy the text not the URL? I now have to use control + C to copy text and cmd + V to paste it? |
@smehrjerdian We will be fixing the bugs that users are running into in a hotfix coming shortly (likely in a day or two). Additionally, we are working on a feature flag that will let users restore the default keyboard shortcuts: brave/brave-browser#29177 A proper UI setting will come later. |
Amazing thanks for the update @fmarier |
This is a substantial UX change from the universal behavior of |
This is still an issue with the new update. The only time |
@0xhashmap What version of the browser are you using? 1.49.132 is the one that has cmd+C mapped to regular copy. |
I updated a few days ago, but I guess 1.49.132 was released since. I'm now on the latest version. Looks like it's fixed. Thank you. |
Resolves brave/brave-browser#26761
Copy Clean Link is default for urls:
Copy text:
Win:
Mac:
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
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Copy clean link
item, try to press Ctrl+c hotkey, it should strip parameters due to copy clean link database. SelectingCopy
item from context menu should copy selected url without changes.Copy
item,Copy clean link
should not be visible