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

Unified Diff: ui/gfx/render_text_win.cc

Issue 252563003: Fix Views inline autocomplete with multi-char graphemes. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Extrapolate on the comment. Created 6 years, 8 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 | « ui/gfx/render_text_win.h ('k') | ui/views/controls/textfield/textfield.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/gfx/render_text_win.cc
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index 46f46dd85c19b79609bc6299fb36d11b9f2d096f..54e3931abb0c36e56ec3c906be4bcc743a957216 100644
--- a/ui/gfx/render_text_win.cc
+++ b/ui/gfx/render_text_win.cc
@@ -504,19 +504,14 @@ HDC RenderTextWin::cached_hdc_ = NULL;
// static
std::map<std::string, Font> RenderTextWin::successful_substitute_fonts_;
-RenderTextWin::RenderTextWin()
- : RenderText(),
- needs_layout_(false) {
+RenderTextWin::RenderTextWin() : RenderText(), needs_layout_(false) {
set_truncate_length(kMaxUniscribeTextLength);
-
memset(&script_control_, 0, sizeof(script_control_));
memset(&script_state_, 0, sizeof(script_state_));
-
MoveCursorTo(EdgeSelectionModel(CURSOR_LEFT));
}
-RenderTextWin::~RenderTextWin() {
-}
+RenderTextWin::~RenderTextWin() {}
Size RenderTextWin::GetStringSize() {
EnsureLayout();
@@ -716,7 +711,7 @@ std::vector<Rect> RenderTextWin::GetSubstringBounds(const Range& range) {
size_t RenderTextWin::TextIndexToLayoutIndex(size_t index) const {
DCHECK_LE(index, text().length());
- ptrdiff_t i = obscured() ? gfx::UTF16IndexToOffset(text(), 0, index) : index;
+ ptrdiff_t i = obscured() ? UTF16IndexToOffset(text(), 0, index) : index;
CHECK_GE(i, 0);
// Clamp layout indices to the length of the text actually used for layout.
return std::min<size_t>(GetLayoutText().length(), i);
@@ -727,24 +722,22 @@ size_t RenderTextWin::LayoutIndexToTextIndex(size_t index) const {
return index;
DCHECK_LE(index, GetLayoutText().length());
- const size_t text_index = gfx::UTF16OffsetToIndex(text(), 0, index);
+ const size_t text_index = UTF16OffsetToIndex(text(), 0, index);
DCHECK_LE(text_index, text().length());
return text_index;
}
-bool RenderTextWin::IsCursorablePosition(size_t position) {
- if (position == 0 || position == text().length())
+bool RenderTextWin::IsValidCursorIndex(size_t index) {
+ if (index == 0 || index == text().length())
return true;
+ if (!IsValidLogicalIndex(index))
+ return false;
EnsureLayout();
-
- // Check that the index is at a valid code point (not mid-surrgate-pair),
- // that it is not truncated from layout text (its glyph is shown on screen),
- // and that its glyph has distinct bounds (not mid-multi-character-grapheme).
- // An example of a multi-character-grapheme that is not a surrogate-pair is:
- // \x0915\x093f - (ki) - one of many Devanagari biconsonantal conjuncts.
- return gfx::IsValidCodePointIndex(text(), position) &&
- position < LayoutIndexToTextIndex(GetLayoutText().length()) &&
- GetGlyphBounds(position) != GetGlyphBounds(position - 1);
+ // Disallow indices amid multi-character graphemes by checking glyph bounds.
+ // These characters are not surrogate-pairs, but may yield a single glyph:
+ // \x0915\x093f - (ki) - one of many Devanagari biconsonantal conjuncts.
+ // \x0e08\x0e33 - (cho chan + sara am) - a Thai consonant and vowel pair.
+ return GetGlyphBounds(index) != GetGlyphBounds(index - 1);
}
void RenderTextWin::ResetLayout() {
@@ -860,8 +853,13 @@ void RenderTextWin::DrawVisualText(Canvas* canvas) {
const Range intersection =
colors().GetRange(it).Intersect(segment->char_range);
const Range colored_glyphs = CharRangeToGlyphRange(*run, intersection);
+ // The range may be empty if a portion of a multi-character grapheme is
+ // selected, yielding two colors for a single glyph. For now, this just
+ // paints the glyph with a single style, but it should paint it twice,
+ // clipped according to selection bounds. See http://crbug.com/366786
+ if (colored_glyphs.is_empty())
+ continue;
DCHECK(glyph_range.Contains(colored_glyphs));
- DCHECK(!colored_glyphs.is_empty());
const SkPoint& start_pos =
pos[colored_glyphs.start() - glyph_range.start()];
const SkPoint& end_pos =
« no previous file with comments | « ui/gfx/render_text_win.h ('k') | ui/views/controls/textfield/textfield.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698