[EditContext] Fix indexing in FirstRectForCharacterRange
The `location` parameter to EditContext::FirstRectForCharacterRange is
relative to the entire contents of the editable region, but
character_bounds_ (if HasValidCompositionBounds() is true) is indexed
only relative to the composition range within the editable region.
So, when indexing into character_bounds_ we must adjust `location` by
the start offset of the editable region.
The `length == 0` cases in FirstRectForCharacterRange were failing
to do this adjustment when indexing into character_bounds_.
Fix the array indexing and add a test.
Bug: 1507725
Change-Id: Ie742cfe0c56fb016d70b32b0f13c4d53e94db2d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5219458
Commit-Queue: Dan Clark <daniec@microsoft.com>
Reviewed-by: Anupam Snigdha <snianu@microsoft.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1251542}
diff --git a/third_party/blink/renderer/core/editing/ime/edit_context.cc b/third_party/blink/renderer/core/editing/ime/edit_context.cc
index b5d06fe..79053d4 100644
--- a/third_party/blink/renderer/core/editing/ime/edit_context.cc
+++ b/third_party/blink/renderer/core/editing/ime/edit_context.cc
@@ -739,29 +739,32 @@
gfx::Rect& rect_in_viewport) {
if (HasValidCompositionBounds()) {
WebRange range = this->CompositionRange();
+ CHECK_GE(range.StartOffset(), 0);
+ CHECK_GE(range.EndOffset(), 0);
// If the requested range is within the current composition range,
// we'll use that to provide the result.
if (base::saturated_cast<int>(location) >= range.StartOffset() &&
base::saturated_cast<int>(location + length) <= range.EndOffset()) {
+ const size_t start_in_composition = location - range.StartOffset();
+ const size_t end_in_composition = location + length - range.StartOffset();
if (length == 0) {
- if (location == character_bounds_.size()) {
+ if (start_in_composition == character_bounds_.size()) {
// Zero-width rect after the last character in the composition range
rect_in_viewport =
- gfx::Rect(character_bounds_[location - 1].right(),
- character_bounds_[location - 1].y(), 0,
- character_bounds_[location - 1].height());
+ gfx::Rect(character_bounds_[start_in_composition - 1].right(),
+ character_bounds_[start_in_composition - 1].y(), 0,
+ character_bounds_[start_in_composition - 1].height());
} else {
// Zero-width rect before the next character in the composition range
- rect_in_viewport = gfx::Rect(character_bounds_[location].x(),
- character_bounds_[location].y(), 0,
- character_bounds_[location].height());
+ rect_in_viewport =
+ gfx::Rect(character_bounds_[start_in_composition].x(),
+ character_bounds_[start_in_composition].y(), 0,
+ character_bounds_[start_in_composition].height());
}
} else {
- const int start_in_composition = location - range.StartOffset();
- const int end_in_composition = location + length - range.StartOffset();
gfx::Rect rect = character_bounds_[start_in_composition];
- for (int i = start_in_composition + 1; i < end_in_composition; ++i) {
+ for (size_t i = start_in_composition + 1; i < end_in_composition; ++i) {
rect.Union(character_bounds_[i]);
}
diff --git a/third_party/blink/web_tests/editing/input/edit-context.html b/third_party/blink/web_tests/editing/input/edit-context.html
index 4d68cd1..feeacd0 100644
--- a/third_party/blink/web_tests/editing/input/edit-context.html
+++ b/third_party/blink/web_tests/editing/input/edit-context.html
@@ -540,6 +540,36 @@
textInputController.setComposition("bar");
}, 'Testing EditContext GC');
+
+test(function() {
+ const editContext = new EditContext();
+ assert_not_equals(editContext, null);
+ const test = document.createElement("div");
+ document.body.appendChild(test);
+ test.editContext = editContext;
+ test.focus();
+ textInputController.insertText("abc")
+ textInputController.setComposition("def");
+ const rect1 = new DOMRect(0, 2, 4, 8);
+ const rect2 = new DOMRect(4, 2, 4, 8);
+ const rect3 = new DOMRect(8, 2, 4, 8);
+ editContext.updateCharacterBounds(3, [rect1, rect2, rect3]);
+ assert_array_equals(textInputController.firstRectForCharacterRange(2, 0), [], "0-width call at index 2 should get empty array");
+ assert_array_equals(textInputController.firstRectForCharacterRange(3, 0), [0, 2, 0, 8], "0-width call at index 3 should get caret position before first character");
+ assert_array_equals(textInputController.firstRectForCharacterRange(4, 0), [4, 2, 0, 8], "0-width call at index 4 should get caret position before second character");
+ assert_array_equals(textInputController.firstRectForCharacterRange(5, 0), [8, 2, 0, 8], "0-width call at index 5 should get caret position before third character");
+ assert_array_equals(textInputController.firstRectForCharacterRange(6, 0), [12, 2, 0, 8], "0-width call at index 6 should get caret position after third character");
+ assert_array_equals(textInputController.firstRectForCharacterRange(7, 0), [], "0-width call at index 7 should get empty array");
+
+ assert_array_equals(textInputController.firstRectForCharacterRange(3, 1), [0, 2, 4, 8], "1-width call at index 3 should get first character rect");
+ assert_array_equals(textInputController.firstRectForCharacterRange(4, 1), [4, 2, 4, 8], "1-width call at index 4 should get second character rect");
+ assert_array_equals(textInputController.firstRectForCharacterRange(5, 1), [8, 2, 4, 8], "1-width call at index 5 should get third character rect");
+ assert_array_equals(textInputController.firstRectForCharacterRange(6, 1), [], "1-width call at index 6 should get empty array");
+
+ assert_array_equals(textInputController.firstRectForCharacterRange(3, 3), [0, 2, 12, 8], "3-width call should join rects for characters 3-5");
+
+ test.remove();
+}, 'Testing firstRectForCharacterRange');
</script>
</body>
</html>
