Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(88)

Unified Diff: ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm

Issue 2956013002: [ios] Workaround error affecting 3rd party keyboards and omnibox. (Closed)
Patch Set: typo Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ios/chrome/browser/experimental_flags.mm ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm
diff --git a/ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm b/ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm
index 2be9472a98248f10a6d12cbd4faf200506c6ab25..ade8d6680099a0fbbc6f290996c919e4d871b424 100644
--- a/ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm
+++ b/ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.mm
@@ -14,7 +14,9 @@
#include "base/strings/sys_string_conversions.h"
#include "components/grit/components_scaled_resources.h"
#include "components/omnibox/browser/autocomplete_input.h"
+#include "ios/chrome/browser/application_context.h"
#include "ios/chrome/browser/autocomplete/autocomplete_scheme_classifier_impl.h"
+#include "ios/chrome/browser/experimental_flags.h"
#import "ios/chrome/browser/ui/animation_util.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_util.h"
#import "ios/chrome/browser/ui/reversed_animation.h"
@@ -458,7 +460,29 @@ NSString* const kOmniboxFadeAnimationKey = @"OmniboxFadeAnimation";
[self clearAutocompleteText];
}
- self.attributedText = fieldText;
+ // The following bool was introduced to workaround a UIKit bug (://32817402).
rohitrao (ping after 24h) 2017/06/28 16:48:23 Is this a b/ url? Should we also link to the rada
rohitrao (ping after 24h) 2017/06/28 16:48:23 BOOL
justincohen 2017/06/28 21:38:06 Done.
justincohen 2017/06/28 21:38:06 The crbug url links to the b/ urls.
+ // The bug relates to third party keyboards that check the value of
+ // textDocumentProxy.documentContextBeforeInput to show keyboard suggestions.
+ // It appears that calling setAttributedText during an EditingChanged
+ // UIControlEvent somehow triggers this bug. The reason we update the
+ // attributed text here is to change the colors of the omnibox (such as host,
+ // protocol) when !self.exiting, but also to hide real UITextField text under
rohitrao (ping after 24h) 2017/06/28 16:48:23 !self.editing. Also in the CL description.
justincohen 2017/06/28 21:38:06 Done.
+ // the _selection text when self.editing. Since we will correct the omnibox
+ // editing text color anytime |self.text| is different than |fieldText|, it
+ // seems it's OK to skip calling self.attributedText during the condition
+ // added below. If we change mobile omnibox to match desktop and also color
+ // the omnibox while self.editing, this workaround will no longer work.
+ BOOL updateText = YES;
+ // Before M61 branch point this should also go behind a Japanese flag, e.g.
+ // [self.textInputMode.primaryLanguage isEqualToString:@"ja-JP"] to be as
+ // restrictive as possible.
+ if (experimental_flags::IsThirdPartyKeyboardWorkaroundEnabled()) {
rohitrao (ping after 24h) 2017/06/28 16:48:23 Is this an expensive call? How about calling it o
justincohen 2017/06/28 21:38:06 It's fast, we call base::FeatureList::IsEnabled a
+ updateText =
+ (!self.editing || ![self.text isEqualToString:fieldText.string]);
+ }
+ if (updateText) {
+ self.attributedText = fieldText;
+ }
// iOS changes the font to .LastResort when some unexpected unicode strings
// are used (e.g. 𝗲𝗺𝗽𝗵𝗮𝘀𝗶𝘀). Setting the NSFontAttributeName in the
« no previous file with comments | « ios/chrome/browser/experimental_flags.mm ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698