diff options
Diffstat (limited to 'ui/gfx')
-rw-r--r-- | ui/gfx/render_text.cc | 52 | ||||
-rw-r--r-- | ui/gfx/render_text.h | 20 | ||||
-rw-r--r-- | ui/gfx/render_text_mac.cc | 14 | ||||
-rw-r--r-- | ui/gfx/render_text_mac.h | 2 | ||||
-rw-r--r-- | ui/gfx/render_text_pango.cc | 37 | ||||
-rw-r--r-- | ui/gfx/render_text_pango.h | 2 | ||||
-rw-r--r-- | ui/gfx/render_text_unittest.cc | 64 | ||||
-rw-r--r-- | ui/gfx/render_text_win.cc | 40 | ||||
-rw-r--r-- | ui/gfx/render_text_win.h | 2 |
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; |