summaryrefslogtreecommitdiffstats
path: root/ui/gfx
diff options
context:
space:
mode:
Diffstat (limited to 'ui/gfx')
-rw-r--r--ui/gfx/render_text.cc52
-rw-r--r--ui/gfx/render_text.h20
-rw-r--r--ui/gfx/render_text_mac.cc14
-rw-r--r--ui/gfx/render_text_mac.h2
-rw-r--r--ui/gfx/render_text_pango.cc37
-rw-r--r--ui/gfx/render_text_pango.h2
-rw-r--r--ui/gfx/render_text_unittest.cc64
-rw-r--r--ui/gfx/render_text_win.cc40
-rw-r--r--ui/gfx/render_text_win.h2
9 files changed, 136 insertions, 97 deletions
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 6455738..37e9721 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -501,26 +501,28 @@ void RenderText::SetCursorPosition(size_t position) {
void RenderText::MoveCursor(BreakType break_type,
VisualCursorDirection direction,
bool select) {
- SelectionModel position(cursor_position(), selection_model_.caret_affinity());
+ SelectionModel cursor(cursor_position(), selection_model_.caret_affinity());
// Cancelling a selection moves to the edge of the selection.
if (break_type != LINE_BREAK && !selection().is_empty() && !select) {
SelectionModel selection_start = GetSelectionModelForSelectionStart();
int start_x = GetCursorBounds(selection_start, true).x();
- int cursor_x = GetCursorBounds(position, true).x();
+ int cursor_x = GetCursorBounds(cursor, true).x();
// Use the selection start if it is left (when |direction| is CURSOR_LEFT)
// or right (when |direction| is CURSOR_RIGHT) of the selection end.
if (direction == CURSOR_RIGHT ? start_x > cursor_x : start_x < cursor_x)
- position = selection_start;
- // For word breaks, use the nearest word boundary in the appropriate
- // |direction|.
+ cursor = selection_start;
+ // Use the nearest word boundary in the proper |direction| for word breaks.
if (break_type == WORD_BREAK)
- position = GetAdjacentSelectionModel(position, break_type, direction);
+ cursor = GetAdjacentSelectionModel(cursor, break_type, direction);
+ // Use an adjacent selection model if the cursor is not at a valid position.
+ if (!IsValidCursorIndex(cursor.caret_pos()))
+ cursor = GetAdjacentSelectionModel(cursor, CHARACTER_BREAK, direction);
} else {
- position = GetAdjacentSelectionModel(position, break_type, direction);
+ cursor = GetAdjacentSelectionModel(cursor, break_type, direction);
}
if (select)
- position.set_selection_start(selection().start());
- MoveCursorTo(position);
+ cursor.set_selection_start(selection().start());
+ MoveCursorTo(cursor);
}
bool RenderText::MoveCursorTo(const SelectionModel& model) {
@@ -528,9 +530,8 @@ bool RenderText::MoveCursorTo(const SelectionModel& model) {
size_t text_length = text().length();
Range range(std::min(model.selection().start(), text_length),
std::min(model.caret_pos(), text_length));
- // The current model only supports caret positions at valid character indices.
- if (!IsCursorablePosition(range.start()) ||
- !IsCursorablePosition(range.end()))
+ // The current model only supports caret positions at valid cursor indices.
+ if (!IsValidCursorIndex(range.start()) || !IsValidCursorIndex(range.end()))
return false;
SelectionModel sel(range, model.caret_affinity());
bool changed = sel != selection_model_;
@@ -548,7 +549,8 @@ bool RenderText::MoveCursorTo(const Point& point, bool select) {
bool RenderText::SelectRange(const Range& range) {
Range sel(std::min(range.start(), text().length()),
std::min(range.end(), text().length()));
- if (!IsCursorablePosition(sel.start()) || !IsCursorablePosition(sel.end()))
+ // Allow selection bounds at valid indicies amid multi-character graphemes.
+ if (!IsValidLogicalIndex(sel.start()) || !IsValidLogicalIndex(sel.end()))
return false;
LogicalCursorDirection affinity =
(sel.is_reversed() || sel.is_empty()) ? CURSOR_FORWARD : CURSOR_BACKWARD;
@@ -767,6 +769,21 @@ void RenderText::DrawCursor(Canvas* canvas, const SelectionModel& position) {
canvas->FillRect(GetCursorBounds(position, true), cursor_color_);
}
+bool RenderText::IsValidLogicalIndex(size_t index) {
+ // Check that the index is at a valid code point (not mid-surrgate-pair) and
+ // that it's not truncated from the layout text (its glyph may be shown).
+ //
+ // Indices within truncated text are disallowed so users can easily interact
+ // with the underlying truncated text using the ellipsis as a proxy. This lets
+ // users select all text, select the truncated text, and transition from the
+ // last rendered glyph to the end of the text without getting invisible cursor
+ // positions nor needing unbounded arrow key presses to traverse the ellipsis.
+ return index == 0 || index == text().length() ||
+ (index < text().length() &&
+ (truncate_length_ == 0 || index < truncate_length_) &&
+ IsValidCodePointIndex(text(), index));
+}
+
Rect RenderText::GetCursorBounds(const SelectionModel& caret,
bool insert_mode) {
// TODO(ckocagil): Support multiline. This function should return the height
@@ -774,9 +791,8 @@ Rect RenderText::GetCursorBounds(const SelectionModel& caret,
// the multiline size, eliminate its use here.
EnsureLayout();
-
size_t caret_pos = caret.caret_pos();
- DCHECK(IsCursorablePosition(caret_pos));
+ DCHECK(IsValidLogicalIndex(caret_pos));
// In overtype mode, ignore the affinity and always indicate that we will
// overtype the next character.
LogicalCursorDirection caret_affinity =
@@ -817,7 +833,7 @@ size_t RenderText::IndexOfAdjacentGrapheme(size_t index,
if (direction == CURSOR_FORWARD) {
while (index < text().length()) {
index++;
- if (IsCursorablePosition(index))
+ if (IsValidCursorIndex(index))
return index;
}
return text().length();
@@ -825,7 +841,7 @@ size_t RenderText::IndexOfAdjacentGrapheme(size_t index,
while (index > 0) {
index--;
- if (IsCursorablePosition(index))
+ if (IsValidCursorIndex(index))
return index;
}
return 0;
@@ -1108,7 +1124,7 @@ bool RenderText::RangeContainsCaret(const Range& range,
void RenderText::MoveCursorTo(size_t position, bool select) {
size_t cursor = std::min(position, text().length());
- if (IsCursorablePosition(cursor))
+ if (IsValidCursorIndex(cursor))
SetSelectionModel(SelectionModel(
Range(select ? selection().start() : cursor, cursor),
(cursor == 0) ? CURSOR_FORWARD : CURSOR_BACKWARD));
diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h
index 93054b4..19c118a 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -378,9 +378,13 @@ class GFX_EXPORT RenderText {
// Gets the SelectionModel from a visual point in local coordinates.
virtual SelectionModel FindCursorPosition(const Point& point) = 0;
- // Return true if cursor can appear in front of the character at |position|,
- // which means it is a grapheme boundary or the first character in the text.
- virtual bool IsCursorablePosition(size_t position) = 0;
+ // Returns true if the position is a valid logical index into text(), and is
+ // also a valid grapheme boundary, which may be used as a cursor position.
+ virtual bool IsValidCursorIndex(size_t index) = 0;
+
+ // Returns true if the position is a valid logical index into text(). Indices
+ // amid multi-character graphemes are allowed here, unlike IsValidCursorIndex.
+ virtual bool IsValidLogicalIndex(size_t index);
// Get the visual bounds of a cursor at |caret|. These bounds typically
// represent a vertical line if |insert_mode| is true. Pass false for
@@ -388,8 +392,7 @@ class GFX_EXPORT RenderText {
// are in local coordinates, but may be outside the visible region if the text
// is longer than the textfield. Subsequent text, cursor, or bounds changes
// may invalidate returned values. Note that |caret| must be placed at
- // grapheme boundary, that is, |IsCursorablePosition(caret.caret_pos())| must
- // return true.
+ // grapheme boundary, i.e. caret.caret_pos() must be a cursorable position.
Rect GetCursorBounds(const SelectionModel& caret, bool insert_mode);
// Compute the current cursor bounds, panning the text to show the cursor in
@@ -398,10 +401,9 @@ class GFX_EXPORT RenderText {
const Rect& GetUpdatedCursorBounds();
// Given an |index| in text(), return the next or previous grapheme boundary
- // in logical order (that is, the nearest index for which
- // |IsCursorablePosition(index)| returns true). The return value is in the
- // range 0 to text().length() inclusive (the input is clamped if it is out of
- // that range). Always moves by at least one character index unless the
+ // in logical order (i.e. the nearest cursorable index). The return value is
+ // in the range 0 to text().length() inclusive (the input is clamped if it is
+ // out of that range). Always moves by at least one character index unless the
// supplied index is already at the boundary of the string.
size_t IndexOfAdjacentGrapheme(size_t index,
LogicalCursorDirection direction);
diff --git a/ui/gfx/render_text_mac.cc b/ui/gfx/render_text_mac.cc
index af0be63..3ad484b 100644
--- a/ui/gfx/render_text_mac.cc
+++ b/ui/gfx/render_text_mac.cc
@@ -45,7 +45,7 @@ std::vector<RenderText::FontSpan> RenderTextMac::GetFontSpansForTesting() {
std::vector<RenderText::FontSpan> spans;
for (size_t i = 0; i < runs_.size(); ++i) {
- gfx::Font font(runs_[i].font_name, runs_[i].text_size);
+ Font font(runs_[i].font_name, runs_[i].text_size);
const CFRange cf_range = CTRunGetStringRange(runs_[i].ct_run);
const Range range(cf_range.location, cf_range.location + cf_range.length);
spans.push_back(RenderText::FontSpan(font, range));
@@ -93,9 +93,9 @@ size_t RenderTextMac::LayoutIndexToTextIndex(size_t index) const {
return index;
}
-bool RenderTextMac::IsCursorablePosition(size_t position) {
+bool RenderTextMac::IsValidCursorIndex(size_t index) {
// TODO(asvitkine): Implement this. http://crbug.com/131618
- return true;
+ return IsValidLogicalIndex(index);
}
void RenderTextMac::ResetLayout() {
@@ -211,7 +211,7 @@ void RenderTextMac::ApplyStyles(CFMutableAttributedStringRef attr_string,
end = TextIndexToLayoutIndex(style.GetRange().end());
const CFRange range = CFRangeMake(i, end - i);
base::ScopedCFTypeRef<CGColorRef> foreground(
- gfx::CGColorCreateFromSkColor(style.color()));
+ CGColorCreateFromSkColor(style.color()));
CFAttributedStringSetAttribute(attr_string, range,
kCTForegroundColorAttributeName, foreground);
CFArrayAppendValue(attributes_, foreground);
@@ -254,9 +254,9 @@ void RenderTextMac::ComputeRuns() {
// TODO(asvitkine): Don't use GetLineOffset() until draw time, since it may be
// updated based on alignment changes without resetting the layout.
- gfx::Vector2d text_offset = GetLineOffset(0);
+ Vector2d text_offset = GetLineOffset(0);
// Skia will draw glyphs with respect to the baseline.
- text_offset += gfx::Vector2d(0, common_baseline_);
+ text_offset += Vector2d(0, common_baseline_);
const SkScalar x = SkIntToScalar(text_offset.x());
const SkScalar y = SkIntToScalar(text_offset.y());
@@ -325,7 +325,7 @@ void RenderTextMac::ComputeRuns() {
base::mac::GetValueFromDictionary<CGColorRef>(
attributes, kCTForegroundColorAttributeName);
if (foreground)
- run->foreground = gfx::CGColorRefToSkColor(foreground);
+ run->foreground = CGColorRefToSkColor(foreground);
const CFNumberRef underline =
base::mac::GetValueFromDictionary<CFNumberRef>(
diff --git a/ui/gfx/render_text_mac.h b/ui/gfx/render_text_mac.h
index 3a18a0f..ad03b84 100644
--- a/ui/gfx/render_text_mac.h
+++ b/ui/gfx/render_text_mac.h
@@ -44,7 +44,7 @@ class RenderTextMac : public RenderText {
virtual std::vector<Rect> GetSubstringBounds(const Range& range) OVERRIDE;
virtual size_t TextIndexToLayoutIndex(size_t index) const OVERRIDE;
virtual size_t LayoutIndexToTextIndex(size_t index) const OVERRIDE;
- virtual bool IsCursorablePosition(size_t position) OVERRIDE;
+ virtual bool IsValidCursorIndex(size_t index) OVERRIDE;
virtual void ResetLayout() OVERRIDE;
virtual void EnsureLayout() OVERRIDE;
virtual void DrawVisualText(Canvas* canvas) OVERRIDE;
diff --git a/ui/gfx/render_text_pango.cc b/ui/gfx/render_text_pango.cc
index fdf3d59..bda684b 100644
--- a/ui/gfx/render_text_pango.cc
+++ b/ui/gfx/render_text_pango.cc
@@ -40,8 +40,7 @@ bool IsForwardMotion(VisualCursorDirection direction, const PangoItem* item) {
}
// Checks whether |range| contains |index|. This is not the same as calling
-// |range.Contains(gfx::Range(index))| - as that would return true when
-// |index| == |range.end()|.
+// range.Contains(Range(index)), which returns true if |index| == |range.end()|.
bool IndexInRange(const Range& range, size_t index) {
return index >= range.start() && index < range.end();
}
@@ -249,7 +248,7 @@ std::vector<Rect> RenderTextPango::GetSubstringBounds(const Range& range) {
size_t RenderTextPango::TextIndexToLayoutIndex(size_t index) const {
DCHECK(layout_);
- ptrdiff_t offset = gfx::UTF16IndexToOffset(text(), 0, index);
+ ptrdiff_t offset = UTF16IndexToOffset(text(), 0, index);
// Clamp layout indices to the length of the text actually used for layout.
offset = std::min<size_t>(offset, g_utf8_strlen(layout_text_, -1));
const char* layout_pointer = g_utf8_offset_to_pointer(layout_text_, offset);
@@ -260,24 +259,19 @@ size_t RenderTextPango::LayoutIndexToTextIndex(size_t index) const {
DCHECK(layout_);
const char* layout_pointer = layout_text_ + index;
const long offset = g_utf8_pointer_to_offset(layout_text_, layout_pointer);
- return gfx::UTF16OffsetToIndex(text(), 0, offset);
+ return UTF16OffsetToIndex(text(), 0, offset);
}
-bool RenderTextPango::IsCursorablePosition(size_t position) {
- if (position == 0 && text().empty())
+bool RenderTextPango::IsValidCursorIndex(size_t index) {
+ if (index == 0 || index == text().length())
return true;
- if (position >= text().length())
- return position == text().length();
- if (!gfx::IsValidCodePointIndex(text(), position))
+ if (!IsValidLogicalIndex(index))
return false;
EnsureLayout();
- ptrdiff_t offset = gfx::UTF16IndexToOffset(text(), 0, position);
- // Check that the index corresponds with a valid text code point, that it is
- // marked as a legitimate cursor position by Pango, and that it is not
- // truncated from layout text (its glyph is shown on screen).
- return (offset < num_log_attrs_ && log_attrs_[offset].is_cursor_position &&
- offset < g_utf8_strlen(layout_text_, -1));
+ ptrdiff_t offset = UTF16IndexToOffset(text(), 0, index);
+ // Check that the index is marked as a legitimate cursor position by Pango.
+ return offset < num_log_attrs_ && log_attrs_[offset].is_cursor_position;
}
void RenderTextPango::ResetLayout() {
@@ -391,11 +385,10 @@ void RenderTextPango::DrawVisualText(Canvas* canvas) {
ApplyTextShadows(&renderer);
// TODO(derat): Use font-specific params: http://crbug.com/125235
- const gfx::FontRenderParams& render_params =
- gfx::GetDefaultFontRenderParams();
+ const FontRenderParams& render_params = GetDefaultFontRenderParams();
const bool use_subpixel_rendering =
render_params.subpixel_rendering !=
- gfx::FontRenderParams::SUBPIXEL_RENDERING_NONE;
+ FontRenderParams::SUBPIXEL_RENDERING_NONE;
renderer.SetFontSmoothingSettings(
render_params.antialiasing,
use_subpixel_rendering && !background_is_transparent(),
@@ -403,16 +396,16 @@ void RenderTextPango::DrawVisualText(Canvas* canvas) {
SkPaint::Hinting skia_hinting = SkPaint::kNormal_Hinting;
switch (render_params.hinting) {
- case gfx::FontRenderParams::HINTING_NONE:
+ case FontRenderParams::HINTING_NONE:
skia_hinting = SkPaint::kNo_Hinting;
break;
- case gfx::FontRenderParams::HINTING_SLIGHT:
+ case FontRenderParams::HINTING_SLIGHT:
skia_hinting = SkPaint::kSlight_Hinting;
break;
- case gfx::FontRenderParams::HINTING_MEDIUM:
+ case FontRenderParams::HINTING_MEDIUM:
skia_hinting = SkPaint::kNormal_Hinting;
break;
- case gfx::FontRenderParams::HINTING_FULL:
+ case FontRenderParams::HINTING_FULL:
skia_hinting = SkPaint::kFull_Hinting;
break;
}
diff --git a/ui/gfx/render_text_pango.h b/ui/gfx/render_text_pango.h
index ba7361c..4c62e0a 100644
--- a/ui/gfx/render_text_pango.h
+++ b/ui/gfx/render_text_pango.h
@@ -36,7 +36,7 @@ class RenderTextPango : public RenderText {
virtual std::vector<Rect> GetSubstringBounds(const Range& range) OVERRIDE;
virtual size_t TextIndexToLayoutIndex(size_t index) const OVERRIDE;
virtual size_t LayoutIndexToTextIndex(size_t index) const OVERRIDE;
- virtual bool IsCursorablePosition(size_t position) OVERRIDE;
+ virtual bool IsValidCursorIndex(size_t index) OVERRIDE;
virtual void ResetLayout() OVERRIDE;
virtual void EnsureLayout() OVERRIDE;
virtual void DrawVisualText(Canvas* canvas) OVERRIDE;
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index cbe3dd5..9cf7bb6 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -44,8 +44,7 @@ const wchar_t kRtlLtrRtl[] = L"\x5d0" L"a" L"\x5d1";
#endif
// Checks whether |range| contains |index|. This is not the same as calling
-// |range.Contains(gfx::Range(index))| - as that would return true when
-// |index| == |range.end()|.
+// range.Contains(Range(index)), which returns true if |index| == |range.end()|.
bool IndexInRange(const Range& range, size_t index) {
return index >= range.start() && index < range.end();
}
@@ -295,9 +294,9 @@ TEST_F(RenderTextTest, ObscuredText) {
EXPECT_EQ(1U, render_text->TextIndexToLayoutIndex(2U));
EXPECT_EQ(0U, render_text->LayoutIndexToTextIndex(0U));
EXPECT_EQ(2U, render_text->LayoutIndexToTextIndex(1U));
- EXPECT_TRUE(render_text->IsCursorablePosition(0U));
- EXPECT_FALSE(render_text->IsCursorablePosition(1U));
- EXPECT_TRUE(render_text->IsCursorablePosition(2U));
+ EXPECT_TRUE(render_text->IsValidCursorIndex(0U));
+ EXPECT_FALSE(render_text->IsValidCursorIndex(1U));
+ EXPECT_TRUE(render_text->IsValidCursorIndex(2U));
// FindCursorPosition() should not return positions between a surrogate pair.
render_text->SetDisplayRect(Rect(0, 0, 20, 20));
@@ -443,11 +442,11 @@ TEST_F(RenderTextTest, ElidedText) {
scoped_ptr<RenderText> expected_render_text(RenderText::CreateInstance());
expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
- expected_render_text->SetDisplayRect(gfx::Rect(0, 0, 9999, 100));
+ expected_render_text->SetDisplayRect(Rect(0, 0, 9999, 100));
scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
render_text->SetFontList(FontList("serif, Sans serif, 12px"));
- render_text->SetElideBehavior(gfx::ELIDE_AT_END);
+ render_text->SetElideBehavior(ELIDE_AT_END);
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
// Compute expected width
@@ -461,7 +460,7 @@ TEST_F(RenderTextTest, ElidedText) {
input.append(WideToUTF16(L" MMMMMMMMMMM"));
render_text->SetText(input);
- render_text->SetDisplayRect(gfx::Rect(0, 0, expected_width, 100));
+ render_text->SetDisplayRect(Rect(0, 0, expected_width, 100));
EXPECT_EQ(input, render_text->text())
<< "->For case " << i << ": " << cases[i].text << "\n";
EXPECT_EQ(WideToUTF16(cases[i].layout_text), render_text->GetLayoutText())
@@ -473,14 +472,14 @@ TEST_F(RenderTextTest, ElidedText) {
TEST_F(RenderTextTest, ElidedObscuredText) {
scoped_ptr<RenderText> expected_render_text(RenderText::CreateInstance());
expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
- expected_render_text->SetDisplayRect(gfx::Rect(0, 0, 9999, 100));
+ expected_render_text->SetDisplayRect(Rect(0, 0, 9999, 100));
expected_render_text->SetText(WideToUTF16(L"**\x2026"));
scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
render_text->SetFontList(FontList("serif, Sans serif, 12px"));
- render_text->SetElideBehavior(gfx::ELIDE_AT_END);
+ render_text->SetElideBehavior(ELIDE_AT_END);
render_text->SetDisplayRect(
- gfx::Rect(0, 0, expected_render_text->GetContentWidth(), 100));
+ Rect(0, 0, expected_render_text->GetContentWidth(), 100));
render_text->SetObscured(true);
render_text->SetText(WideToUTF16(L"abcdef"));
EXPECT_EQ(WideToUTF16(L"abcdef"), render_text->text());
@@ -892,26 +891,58 @@ TEST_F(RenderTextTest, GraphemePositions) {
{ kText3, 50, 6, 6 },
};
- // TODO(asvitkine): Disable tests that fail on XP bots due to lack of complete
- // font support for some scripts - http://crbug.com/106450
#if defined(OS_WIN)
+ // TODO(msw): XP fails due to lack of font support: http://crbug.com/106450
if (base::win::GetVersion() < base::win::VERSION_VISTA)
return;
#endif
scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
+ SCOPED_TRACE(base::StringPrintf("Testing cases[%" PRIuS "]", i));
render_text->SetText(cases[i].text);
size_t next = render_text->IndexOfAdjacentGrapheme(cases[i].index,
CURSOR_FORWARD);
EXPECT_EQ(cases[i].expected_next, next);
- EXPECT_TRUE(render_text->IsCursorablePosition(next));
+ EXPECT_TRUE(render_text->IsValidCursorIndex(next));
size_t previous = render_text->IndexOfAdjacentGrapheme(cases[i].index,
CURSOR_BACKWARD);
EXPECT_EQ(cases[i].expected_previous, previous);
- EXPECT_TRUE(render_text->IsCursorablePosition(previous));
+ EXPECT_TRUE(render_text->IsValidCursorIndex(previous));
+ }
+}
+
+TEST_F(RenderTextTest, MidGraphemeSelectionBounds) {
+#if defined(OS_WIN)
+ // TODO(msw): XP fails due to lack of font support: http://crbug.com/106450
+ if (base::win::GetVersion() < base::win::VERSION_VISTA)
+ return;
+#endif
+
+ // Test that selection bounds may be set amid multi-character graphemes.
+ const base::string16 kHindi = WideToUTF16(L"\x0915\x093f");
+ const base::string16 kThai = WideToUTF16(L"\x0e08\x0e33");
+ const base::string16 cases[] = { kHindi, kThai };
+
+ scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
+ for (size_t i = 0; i < arraysize(cases); i++) {
+ SCOPED_TRACE(base::StringPrintf("Testing cases[%" PRIuS "]", i));
+ render_text->SetText(cases[i]);
+ EXPECT_TRUE(render_text->IsValidLogicalIndex(1));
+#if !defined(OS_MACOSX)
+ EXPECT_FALSE(render_text->IsValidCursorIndex(1));
+#endif
+ EXPECT_TRUE(render_text->SelectRange(Range(2, 1)));
+ EXPECT_EQ(Range(2, 1), render_text->selection());
+ EXPECT_EQ(1U, render_text->cursor_position());
+ // Although selection bounds may be set within a multi-character grapheme,
+ // cursor movement (e.g. via arrow key) should avoid those indices.
+ render_text->MoveCursor(CHARACTER_BREAK, CURSOR_LEFT, false);
+ EXPECT_EQ(0U, render_text->cursor_position());
+ render_text->MoveCursor(CHARACTER_BREAK, CURSOR_RIGHT, false);
+ EXPECT_EQ(2U, render_text->cursor_position());
}
}
@@ -941,9 +972,8 @@ TEST_F(RenderTextTest, EdgeSelectionModels) {
{ kHebrewLatin, base::i18n::RIGHT_TO_LEFT },
};
- // TODO(asvitkine): Disable tests that fail on XP bots due to lack of complete
- // font support for some scripts - http://crbug.com/106450
#if defined(OS_WIN)
+ // TODO(msw): XP fails due to lack of font support: http://crbug.com/106450
if (base::win::GetVersion() < base::win::VERSION_VISTA)
return;
#endif
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index 46f46dd..54e3931 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 =
diff --git a/ui/gfx/render_text_win.h b/ui/gfx/render_text_win.h
index 1a1ba48..4e8caf6 100644
--- a/ui/gfx/render_text_win.h
+++ b/ui/gfx/render_text_win.h
@@ -80,7 +80,7 @@ class RenderTextWin : public RenderText {
virtual std::vector<Rect> GetSubstringBounds(const Range& range) OVERRIDE;
virtual size_t TextIndexToLayoutIndex(size_t index) const OVERRIDE;
virtual size_t LayoutIndexToTextIndex(size_t index) const OVERRIDE;
- virtual bool IsCursorablePosition(size_t position) OVERRIDE;
+ virtual bool IsValidCursorIndex(size_t index) OVERRIDE;
virtual void ResetLayout() OVERRIDE;
virtual void EnsureLayout() OVERRIDE;
virtual void DrawVisualText(Canvas* canvas) OVERRIDE;