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

Unified Diff: chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm

Issue 2395233005: [Mac] Preserve original selection when suggesting completions with diacritics (Closed)
Patch Set: Wrap retained object in scoped_nsobject Created 4 years, 2 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
Index: chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
diff --git a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
index 83126cf2e5eee10622c834fac7b0c7ca7142aed2..9bda172a88655fd73100db09fc64b83c51c2d557 100644
--- a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
+++ b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
@@ -4,6 +4,9 @@
#import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.h"
+#include "base/i18n/break_iterator.h"
+#include "base/mac/foundation_util.h"
+#include "base/mac/scoped_nsobject.h"
#include "base/mac/sdk_forward_declarations.h"
#include "base/strings/string_util.h"
#include "base/strings/sys_string_conversions.h"
@@ -49,13 +52,58 @@ BOOL ThePasteboardIsTooDamnBig() {
return [[pb stringForType:type] length] > kMaxPasteLength;
}
+NSRange VisualSelectedRangeFromRange(NSRange range, NSString* string) {
+ if (range.location >= string.length)
+ return range;
+ base::string16 text = base::SysNSStringToUTF16(string);
+ base::i18n::BreakIterator grapheme_iterator(
+ text, base::i18n::BreakIterator::BREAK_CHARACTER);
+ if (!grapheme_iterator.Init())
+ return range;
+ // This works because NSString uses UTF-16 code units.
+ while (range.location < text.length() &&
+ !grapheme_iterator.IsGraphemeBoundary(
+ static_cast<size_t>(range.location))) {
+ range.location++;
+ if (range.length > 0)
+ range.length--;
+ }
+ return range;
+}
+
} // namespace
+// Method exposed for the purpose of overriding.
+// Used to restore model's selection range when the view doesn't
+// match the model due to combining characters.
+//
+// In some cases, (completing 'y' to 'ÿour', for example), the autocomplete
+// system requests a selection range that begins on a combining character.
+// setSelectedRange: and friends document that the range passed to them
+// "must begin and end on glyph boundaries and not split base glyphs and
+// their nonspacing marks". If passed such a range, the selection is
+// expanded to include the original user input, preventing the user
+// from being able to type other words beginning with that letter.
+//
+// To resolve this, the field editor modifies the selection to start
+// on the next glyph boundary, then keeps track of the original and
+// modified selections, substituting the original when the user takes
+// actions that operate on the selection. Since there are many methods
+// in NSResponder (for example deleteToBeginningOfLine:) that operate
+// on the selection, rather than shimming them all, we override this
+// private method that they're implemented in terms of.
+//
+@interface NSTextView (PrivateTextEditing)
+- (void)_userReplaceRange:(NSRange)range withString:(NSString*)string;
+@end
+
@interface AutocompleteTextFieldEditor ()<NSDraggingSource>
@end
@implementation AutocompleteTextFieldEditor
+@synthesize actualSelectedRange = actualSelectedRange_;
+
- (BOOL)shouldDrawInsertionPoint {
return [super shouldDrawInsertionPoint] &&
![[[self delegate] cell] hideFocusState];
@@ -400,6 +448,14 @@ BOOL ThePasteboardIsTooDamnBig() {
return [dropHandler_ performDragOperation:sender];
}
+- (void)_userReplaceRange:(NSRange)range withString:(NSString*)string {
+ if (NSEqualRanges(visualSelectedRange_, range) &&
+ !NSEqualRanges(visualSelectedRange_, actualSelectedRange_)) {
+ range = actualSelectedRange_;
+ }
+ [super _userReplaceRange:range withString:string];
+}
+
// Prevent control characters from being entered into the Omnibox.
// This is invoked for keyboard entry, not for pasting.
- (void)insertText:(id)aString {
@@ -431,11 +487,15 @@ BOOL ThePasteboardIsTooDamnBig() {
DCHECK_EQ(range.length, 0U);
}
- // NOTE: If |aString| is empty, this intentionally replaces the
- // selection with empty. This seems consistent with the case where
- // the input contained a mixture of characters and the string ended
- // up not empty.
- [super insertText:aString];
+ if (!NSEqualRanges(visualSelectedRange_, actualSelectedRange_)) {
+ [super replaceCharactersInRange:actualSelectedRange_ withString:aString];
+ } else {
+ // NOTE: If |aString| is empty, this intentionally replaces the
+ // selection with empty. This seems consistent with the case where
+ // the input contained a mixture of characters and the string ended
+ // up not empty.
+ [super insertText:aString];
+ }
}
- (NSRange)selectionRangeForProposedRange:(NSRange)proposedSelRange
@@ -459,6 +519,24 @@ BOOL ThePasteboardIsTooDamnBig() {
textChangedByKeyEvents_ = YES;
}
+- (void)setSelectedRanges:(NSArray*)ranges
+ affinity:(NSSelectionAffinity)affinity
+ stillSelecting:(BOOL)flag {
+ DCHECK(ranges.count > 0);
+ base::scoped_nsobject<NSMutableArray> mutableRanges([ranges mutableCopy]);
+ // |ranges| is sorted, and empirically, the first range passed is returned
+ // as selectedRange.
+ NSRange firstRange = [base::mac::ObjCCastStrict<NSValue>(
+ [mutableRanges firstObject]) rangeValue];
+ actualSelectedRange_ = firstRange;
+ visualSelectedRange_ =
+ VisualSelectedRangeFromRange(firstRange, [self string]);
+ NSValue* boxedVisualRange = [NSValue valueWithRange:visualSelectedRange_];
+ [mutableRanges replaceObjectAtIndex:0 withObject:boxedVisualRange];
+
+ [super setSelectedRanges:mutableRanges affinity:affinity stillSelecting:flag];
+}
+
- (void)interpretKeyEvents:(NSArray *)eventArray {
DCHECK(!interpretingKeyEvents_);
interpretingKeyEvents_ = YES;
@@ -564,7 +642,7 @@ BOOL ThePasteboardIsTooDamnBig() {
- (BOOL)validateMenuItem:(NSMenuItem*)item {
if ([item action] == @selector(copyToFindPboard:))
- return [self selectedRange].length > 0;
+ return actualSelectedRange_.length > 0;
if ([item action] == @selector(pasteAndGo:)) {
// TODO(rohitrao): If the clipboard is empty, should we show a
// greyed-out "Paste and Go" or nothing at all?
@@ -576,11 +654,10 @@ BOOL ThePasteboardIsTooDamnBig() {
}
- (void)copyToFindPboard:(id)sender {
- NSRange selectedRange = [self selectedRange];
- if (selectedRange.length == 0)
+ if (actualSelectedRange_.length == 0)
return;
NSAttributedString* selection =
- [self attributedSubstringForProposedRange:selectedRange
+ [self attributedSubstringForProposedRange:actualSelectedRange_
actualRange:NULL];
if (!selection)
return;

Powered by Google App Engine
This is Rietveld 408576698