Commit 5f5a597f authored by Nektarios Paisios's avatar Nektarios Paisios Committed by Commit Bot

Ensures that both selection and line boundaries work across images

When a replaced layout object, such as an image, was used inside editable text
navigating by line was broken.
Also added test coverage for selecting across an image which I believe was not working
properly with the old selection code.
R=dmazzoni@chromium.org, dtseng@chromium.org

Change-Id: Idea8b11d7d2be90ecaf4069e80de7dd426275e62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1577971
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653435}
parent 86c8bca8
...@@ -587,7 +587,7 @@ TEST_F('ChromeVoxEditingTest', 'RichTextImageByCharacter', function() { ...@@ -587,7 +587,7 @@ TEST_F('ChromeVoxEditingTest', 'RichTextImageByCharacter', function() {
var mockFeedback = this.createMockFeedback(); var mockFeedback = this.createMockFeedback();
this.runWithLoadedTree(function() {/*! this.runWithLoadedTree(function() {/*!
<p contenteditable> <p contenteditable>
<img alt="dog"></img> is a <img alt="cat"</img> test <img alt="dog"> is a <img alt="cat"> test
</p> </p>
<button id="go">Go</button> <button id="go">Go</button>
<script> <script>
...@@ -604,7 +604,7 @@ TEST_F('ChromeVoxEditingTest', 'RichTextImageByCharacter', function() { ...@@ -604,7 +604,7 @@ TEST_F('ChromeVoxEditingTest', 'RichTextImageByCharacter', function() {
this.listenOnce(input, 'focus', function() { this.listenOnce(input, 'focus', function() {
var lineText = 'dog is a cat test mled'; var lineText = 'dog is a cat test mled';
var lineOnDogText = 'dog img is a cat test mled'; var lineOnDogText = 'dog img is a cat test mled';
var lineOnCatText = 'dog is a cat img test'; var lineOnCatText = 'dog is a cat img test mled';
mockFeedback mockFeedback
// This is initial output from focusing the contenteditable (which has // This is initial output from focusing the contenteditable (which has
// no role). // no role).
......
...@@ -168,27 +168,27 @@ Node* AXInlineTextBox::GetNode() const { ...@@ -168,27 +168,27 @@ Node* AXInlineTextBox::GetNode() const {
} }
AXObject* AXInlineTextBox::NextOnLine() const { AXObject* AXInlineTextBox::NextOnLine() const {
if (inline_text_box_->IsLast())
return ParentObject()->NextOnLine();
scoped_refptr<AbstractInlineTextBox> next_on_line = scoped_refptr<AbstractInlineTextBox> next_on_line =
inline_text_box_->NextOnLine(); inline_text_box_->NextOnLine();
if (next_on_line) if (next_on_line)
return ax_object_cache_->GetOrCreate(next_on_line.get()); return ax_object_cache_->GetOrCreate(next_on_line.get());
if (!inline_text_box_->IsLast()) return nullptr;
return nullptr;
return ParentObject()->NextOnLine();
} }
AXObject* AXInlineTextBox::PreviousOnLine() const { AXObject* AXInlineTextBox::PreviousOnLine() const {
if (inline_text_box_->IsFirst())
return ParentObject()->PreviousOnLine();
scoped_refptr<AbstractInlineTextBox> previous_on_line = scoped_refptr<AbstractInlineTextBox> previous_on_line =
inline_text_box_->PreviousOnLine(); inline_text_box_->PreviousOnLine();
if (previous_on_line) if (previous_on_line)
return ax_object_cache_->GetOrCreate(previous_on_line.get()); return ax_object_cache_->GetOrCreate(previous_on_line.get());
if (!inline_text_box_->IsFirst()) return nullptr;
return nullptr;
return ParentObject()->PreviousOnLine();
} }
} // namespace blink } // namespace blink
...@@ -69,6 +69,7 @@ ...@@ -69,6 +69,7 @@
#include "third_party/blink/renderer/core/input_type_names.h" #include "third_party/blink/renderer/core/input_type_names.h"
#include "third_party/blink/renderer/core/layout/api/line_layout_api_shim.h" #include "third_party/blink/renderer/core/layout/api/line_layout_api_shim.h"
#include "third_party/blink/renderer/core/layout/hit_test_result.h" #include "third_party/blink/renderer/core/layout/hit_test_result.h"
#include "third_party/blink/renderer/core/layout/layout_box.h"
#include "third_party/blink/renderer/core/layout/layout_file_upload_control.h" #include "third_party/blink/renderer/core/layout/layout_file_upload_control.h"
#include "third_party/blink/renderer/core/layout/layout_html_canvas.h" #include "third_party/blink/renderer/core/layout/layout_html_canvas.h"
#include "third_party/blink/renderer/core/layout/layout_image.h" #include "third_party/blink/renderer/core/layout/layout_image.h"
...@@ -76,6 +77,7 @@ ...@@ -76,6 +77,7 @@
#include "third_party/blink/renderer/core/layout/layout_list_item.h" #include "third_party/blink/renderer/core/layout/layout_list_item.h"
#include "third_party/blink/renderer/core/layout/layout_list_marker.h" #include "third_party/blink/renderer/core/layout/layout_list_marker.h"
#include "third_party/blink/renderer/core/layout/layout_menu_list.h" #include "third_party/blink/renderer/core/layout/layout_menu_list.h"
#include "third_party/blink/renderer/core/layout/layout_replaced.h"
#include "third_party/blink/renderer/core/layout/layout_table.h" #include "third_party/blink/renderer/core/layout/layout_table.h"
#include "third_party/blink/renderer/core/layout/layout_table_cell.h" #include "third_party/blink/renderer/core/layout/layout_table_cell.h"
#include "third_party/blink/renderer/core/layout/layout_table_row.h" #include "third_party/blink/renderer/core/layout/layout_table_row.h"
...@@ -1266,7 +1268,9 @@ AXObject* AXLayoutObject::NextOnLine() const { ...@@ -1266,7 +1268,9 @@ AXObject* AXLayoutObject::NextOnLine() const {
result = NextOnLineInternalNG(*this); result = NextOnLineInternalNG(*this);
} else { } else {
InlineBox* inline_box = nullptr; InlineBox* inline_box = nullptr;
if (GetLayoutObject()->IsLayoutInline()) { if (GetLayoutObject()->IsBox()) {
inline_box = ToLayoutBox(GetLayoutObject())->InlineBoxWrapper();
} else if (GetLayoutObject()->IsLayoutInline()) {
inline_box = ToLayoutInline(GetLayoutObject())->LastLineBox(); inline_box = ToLayoutInline(GetLayoutObject())->LastLineBox();
} else if (GetLayoutObject()->IsText()) { } else if (GetLayoutObject()->IsText()) {
inline_box = ToLayoutText(GetLayoutObject())->LastTextBox(); inline_box = ToLayoutText(GetLayoutObject())->LastTextBox();
...@@ -1339,7 +1343,9 @@ AXObject* AXLayoutObject::PreviousOnLine() const { ...@@ -1339,7 +1343,9 @@ AXObject* AXLayoutObject::PreviousOnLine() const {
result = PreviousOnLineInlineNG(*this); result = PreviousOnLineInlineNG(*this);
} else { } else {
InlineBox* inline_box = nullptr; InlineBox* inline_box = nullptr;
if (GetLayoutObject()->IsLayoutInline()) { if (GetLayoutObject()->IsBox()) {
inline_box = ToLayoutBox(GetLayoutObject())->InlineBoxWrapper();
} else if (GetLayoutObject()->IsLayoutInline()) {
inline_box = ToLayoutInline(GetLayoutObject())->FirstLineBox(); inline_box = ToLayoutInline(GetLayoutObject())->FirstLineBox();
} else if (GetLayoutObject()->IsText()) { } else if (GetLayoutObject()->IsText()) {
inline_box = ToLayoutText(GetLayoutObject())->FirstTextBox(); inline_box = ToLayoutText(GetLayoutObject())->FirstTextBox();
......
...@@ -1571,7 +1571,11 @@ TEST_F(AccessibilitySelectionTest, List) { ...@@ -1571,7 +1571,11 @@ TEST_F(AccessibilitySelectionTest, List) {
RunSelectionTest("list"); RunSelectionTest("list");
} }
TEST_F(AccessibilitySelectionTest, table) { TEST_F(AccessibilitySelectionTest, SVG) {
RunSelectionTest("svg");
}
TEST_F(AccessibilitySelectionTest, Table) {
RunSelectionTest("table"); RunSelectionTest("table");
} }
......
================================================================================
AXSelection from AX text position in "StaticText": "Some text. ", 0 to AX text position in "StaticText": " More text.", 0
================================================================================
++<GenericContainer>
++++<Paragraph>
^++++++<StaticText: ^Some text. >
++++++<Image: Square>
|++++++<StaticText: | More text.>
================================================================================
AXSelection from AX text position in "StaticText": "Some text. ", 10 to AX text position in "StaticText": " More text.", 1
================================================================================
++<GenericContainer>
++++<Paragraph>
++++++<StaticText: Some text.^ >
++++++<Image: Square>
++++++<StaticText: |More text.>
================================================================================
AXSelection from AX text position in "StaticText": "Some text. ", 11 to AX text position in "StaticText": " More text.", 11
================================================================================
++<GenericContainer>
++++<Paragraph>
++++++<StaticText: Some text. ^>
++++++<Image: Square>
++++++<StaticText: More text.|>
<div contenteditable>
<p>^Some text.^
^<img alt="Square"
src="data:image/svg+xml;utf8,
<svg xmlns='http://www.w3.org/2000/svg'
width='9' height='9'>
<rect width='9' height='9' fill='green'/>
</svg>">|
|More text.|
</p>
</div>
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<div contenteditable>
<p id="paragraph">Some text.
<img alt="Square"
src="data:image/svg+xml;utf8,
<svg xmlns='http://www.w3.org/2000/svg'
width='9' height='9'>
<rect width='9' height='9' fill='green'/>
</svg>">
More text.
</p>
</div>
<script>
test(() => {
let axParagraph = accessibilityController.accessibleElementById('paragraph');
assert_not_equals(axParagraph, undefined);
assert_equals(axParagraph.childrenCount, 3);
let axText1 = axParagraph.childAtIndex(0);
assert_not_equals(axText1, undefined);
assert_equals(axText1.role, 'AXRole: AXStaticText');
let axInline1 = axText1.childAtIndex(0);
assert_not_equals(axInline1, undefined);
assert_equals(axInline1.role, 'AXRole: AXInlineTextBox');
assert_equals(axInline1.name, 'Some text. ');
let axSvg = axParagraph.childAtIndex(1);
assert_not_equals(axSvg, undefined);
assert_equals(axSvg.role, 'AXRole: AXImage');
let axText2 = axParagraph.childAtIndex(2);
assert_not_equals(axText2, undefined);
assert_equals(axText2.role, 'AXRole: AXStaticText');
let axInline2= axText2.childAtIndex(0);
assert_not_equals(axInline2, undefined);
assert_equals(axInline2.role, 'AXRole: AXInlineTextBox');
assert_equals(axInline2.name, ' ');
let axInline3 = axText2.childAtIndex(1);
assert_not_equals(axInline3, undefined);
assert_equals(axInline3.role, 'AXRole: AXInlineTextBox');
assert_equals(axInline3.name, 'More text.');
assert_equals(axText1.previousOnLine(), undefined, 'axText1.previousOnLine()');
assert_equals(axText1.nextOnLine(), axSvg, 'axText1.nextOnLine()');
assert_equals(axInline1.previousOnLine(), undefined, 'axInline1.previousOnLine()');
assert_equals(axInline1.nextOnLine(), axSvg, 'axInline1.nextOnLine()');
assert_equals(axSvg.previousOnLine(), axInline1, 'axSvg.previousOnLine()');
assert_equals(axSvg.nextOnLine(), axInline2, 'axSvg.nextOnLine()');
assert_equals(axText2.previousOnLine(), axSvg, 'axText2.previousOnLine()');
assert_equals(axText2.nextOnLine(), undefined, 'axText2.nextOnLine()');
assert_equals(axInline2.previousOnLine(), axSvg, 'axInline2.previousOnLine()');
assert_equals(axInline2.nextOnLine(), axInline3, 'axInline2.nextOnLine()');
assert_equals(axInline3.previousOnLine(), axInline2, 'axInline3.previousOnLine()');
assert_equals(axInline3.nextOnLine(), undefined, 'axInline3.nextOnLine()');
}, 'Test that the SVG is on the same line as the static text surrounding it.');
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment