Skip to content
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

Avoid clobering user input on newtab #2222

Merged
merged 2 commits into from
Apr 17, 2019
Merged

Avoid clobering user input on newtab #2222

merged 2 commits into from
Apr 17, 2019

Conversation

bbondy
Copy link
Member

@bbondy bbondy commented Apr 15, 2019

This just pulls in a fix that was done upstream in C75 into C74.
The upstream fix will happen in 75.0.3760.0, at which point we can drop these patches.

Fix brave/brave-browser#3756

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) on
    • Windows
    • macOS
    • Linux
  • Verified that all lint errors/warnings are resolved (npm run lint)
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@bbondy bbondy added this to the 0.65.x - Nightly milestone Apr 15, 2019
@bbondy bbondy self-assigned this Apr 15, 2019
@bbondy bbondy changed the title Location bar focus Avoid clobering user input on newtab Apr 15, 2019
emerick
emerick previously approved these changes Apr 15, 2019
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -2,7 +2,7 @@ diff --git a/chrome/browser/global_keyboard_shortcuts_mac.mm b/chrome/browser/gl
index 733785418f6e0fd4090f2c5b82517d01982ab8a5..7b8a57bc712fe2ff3c82ad35f8261cc5451afea0 100644
--- a/chrome/browser/global_keyboard_shortcuts_mac.mm
+++ b/chrome/browser/global_keyboard_shortcuts_mac.mm
@@ -166,6 +166,7 @@ const std::vector<KeyboardShortcutData>& GetShortcutsNotPresentInMainMenu() {
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was in a different commit which is because the previous Chrome rebase did not rebase these files properly.

@@ -2,7 +2,7 @@ diff --git a/chrome/browser/notifications/notification_platform_bridge_mac.mm b/
index 08078c5f168ebf562fd19c0ebd621f1f7a5a6b5d..8bcd16b11888402fa48926564e13f78657442a05 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -226,6 +226,7 @@ void NotificationPlatformBridgeMac::Display(
@@ -226,6 +226,7 @@ @interface AlertDispatcherImpl : NSObject<AlertDispatcher>
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

@bbondy bbondy Apr 15, 2019

Choose a reason for hiding this comment

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

See above.

@@ -2,7 +2,7 @@ diff --git a/chrome/browser/ui/cocoa/accelerators_cocoa.mm b/chrome/browser/ui/c
index 37baab628592ff2a2ee93f8af0c2e687d35bab42..2145c7b6463628dcea7e0a8b4826d90e00b251d0 100644
--- a/chrome/browser/ui/cocoa/accelerators_cocoa.mm
+++ b/chrome/browser/ui/cocoa/accelerators_cocoa.mm
@@ -40,6 +40,8 @@ const struct AcceleratorMapping {
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

@bbondy bbondy Apr 15, 2019

Choose a reason for hiding this comment

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

see above.

@@ -2,7 +2,7 @@ diff --git a/components/crash/content/app/crashpad_mac.mm b/components/crash/con
index a76d264648d23e7e228dd54f00a01035a29a76ad..ee7d94be5982353fced6174afef990713228af37 100644
--- a/components/crash/content/app/crashpad_mac.mm
+++ b/components/crash/content/app/crashpad_mac.mm
@@ -136,6 +136,8 @@ base::FilePath PlatformCrashpadInitialization(
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

@@ -10,7 +10,7 @@ index 73cd30591119969e5de1b2de56b984e54cdf6bac..4d14a957e73354cf79fabb0bb1be9bda
#include "base/mac/mac_logging.h"
#include "base/mac/scoped_cftyperef.h"
#include "base/rand_util.h"
@@ -51,8 +52,8 @@ std::string AddRandomPasswordToKeychain(const AppleKeychain& keychain,
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

@@ -21,7 +21,7 @@ index 73cd30591119969e5de1b2de56b984e54cdf6bac..4d14a957e73354cf79fabb0bb1be9bda
#endif

KeychainPassword::KeychainPassword(
@@ -63,31 +64,50 @@ KeychainPassword::KeychainPassword(
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

@@ -2,7 +2,7 @@ diff --git a/components/os_crypt/os_crypt_mac.mm b/components/os_crypt/os_crypt_
index 88304b2c3865ea3b126789f6a976d8bae4aa50ba..43dc62d9b829dbfbb605e7bfbf30cc0180daafc5 100644
--- a/components/os_crypt/os_crypt_mac.mm
+++ b/components/os_crypt/os_crypt_mac.mm
@@ -100,6 +100,10 @@ crypto::SymmetricKey* GetEncryptionKey() {
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

@@ -2,7 +2,7 @@ diff --git a/ui/native_theme/native_theme_mac.mm b/ui/native_theme/native_theme_
index de3389fac98a708420374851dbf7c0970cb68102..32ca3fede8784b0f519174f314c302761dd0be75 100644
--- a/ui/native_theme/native_theme_mac.mm
+++ b/ui/native_theme/native_theme_mac.mm
@@ -55,7 +55,11 @@
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

pilgrim-brave
pilgrim-brave previously approved these changes Apr 16, 2019
Fix brave/brave-browser#3756

This implements a fix which is already in Chromium 75. Once we get on Chromium 75 we can drop all of these patches.  The fix this ports is here: https://chromium.googlesource.com/chromium/src/+/43ef038d92433b1e8230882d3daac767481d29d9
@bbondy bbondy dismissed stale reviews from pilgrim-brave and emerick via 32211ab April 16, 2019 16:01
@bbondy bbondy merged commit 7fcd14f into master Apr 17, 2019
bbondy added a commit that referenced this pull request Apr 17, 2019
Avoid clobering user input on newtab
bbondy added a commit that referenced this pull request Apr 17, 2019
Avoid clobering user input on newtab
@mihaiplesa mihaiplesa deleted the location-bar-focus branch May 13, 2019 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When typing in url bar in new tab, first few characters of url get deleted
3 participants