chromium/src
0
Fork 0
Code Activity

Fix AXLayoutObject::IsEditable for selectors (e.g., ::before)

Browse Source 
For a specific range, AXPlatformNodeTextRangeProvider::GetAttributeValue
returns the attribute value found in all of its subranges. For example,
if we query the read-only attribute for a range composed of two
subranges, we are going to query the read-only attribute of the two
different subrange. If the value is true in both subranges, true will be
returned. Otherwise, if the value is different, an unknown value will be
returned.

An issue exists with rich text fields. Because the rich text field
accessible node can have children, it can have subranges. That means
that, when we query the read-only attribute for a rich text field, we
look for the read-only value of all of its children. It is thus extra
important that the appropriate properties describing the editable state
of the text field node are propagated to its children node.

Working on this issue, I discovered that the editable properties are not
propagated to its children that are selectors, like the ::before
selector. It works fine for other children however.

This CL fixes this issue by modifying the implementation of
AXLayoutObject::GetNodeOrContainingBlockNode, called by
AXLayoutObject::IsEditable and AXLayoutObject::IsRichlyEditable.

I also took the liberty to fix a test I added in another CL related to
the read-only attribute. The test in itself had no issue for the
moment, but would have as soon as someone else would have modified it.
The error message was not helpful and I spent some time investigating it.
To avoid to waste someone else's time like I did, I fixed it. For more
info, see the changes made to
ax_platform_node_textrangeprovider_win_unittest.cc.

In short:
- We now pass down the editable properties all children of an editable
  node.
- I added a dump test for this scenario.
- I fixed a future issue that would have happened with a unit test.
- I made FindNonAnonymousContainingBlock public and move the
  implementation accordingly.


Bug: 1041305
Change-Id: I3ef0af27f0ef679eb27cf12ce5999c6c11fffccf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1996269
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739533}
This commit is contained in:
Benjamin Beaudry
2020-02-07 20:33:32 +00:00
committed by Commit Bot
parent 56bcbbe34c
commit 087b684a6e
5 changed files with 73 additions and 24 deletions
content/browser/accessibility
ax_platform_node_textrangeprovider_win_browsertest.cc
third_party/blink/renderer
core
layout
layout_object.cclayout_object.h
modules
accessibility
ax_layout_object.cc
ui/accessibility/platform
ax_platform_node_textrangeprovider_win_unittest.cc

36
content/browser/accessibility/ax_platform_node_textrangeprovider_win_browsertest.cc

@@ -479,6 +479,42 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
value.Reset(); value.Reset();
} }
// With a rich text field, the read-only attribute should be determined based on
// the editable root node's editable state.
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
GetAttributeValueIsReadonlyRichTextField) {
LoadInitialAccessibilityTreeFromHtml(std::string(R"HTML(
<!DOCTYPE html>
<html>
<style>
.myDiv::before {
content: attr(data-placeholder);
pointer-events: none;
}
</style>
<body>
<div contenteditable="true" data-placeholder="@mention or comment"
role="textbox" aria-readonly="false" aria-label="text_field"
class="myDiv"><p>3.14</p></div>
</body>
</html>
)HTML"));
auto* text_field_node = FindNode(ax::mojom::Role::kTextField, "text_field");
ASSERT_NE(nullptr, text_field_node);
ComPtr<ITextRangeProvider> text_range_provider;
GetTextRangeProviderFromTextNode(*text_field_node, &text_range_provider);
ASSERT_NE(nullptr, text_range_provider.Get());
base::win::ScopedVariant value;
EXPECT_HRESULT_SUCCEEDED(text_range_provider->GetAttributeValue(
UIA_IsReadOnlyAttributeId, value.Receive()));
EXPECT_EQ(value.type(), VT_BOOL);
EXPECT_EQ(V_BOOL(value.ptr()), VARIANT_FALSE);
text_range_provider.Reset();
value.Reset();
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest, IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
GetBoundingRectangles) { GetBoundingRectangles) {
LoadInitialAccessibilityTreeFromHtml(std::string(R"HTML( LoadInitialAccessibilityTreeFromHtml(std::string(R"HTML(

36
third_party/blink/renderer/core/layout/layout_object.cc vendored

@@ -1226,24 +1226,6 @@ LayoutObject* LayoutObject::ContainerForFixedPosition(
}); });
} }
LayoutBlock* LayoutObject::FindNonAnonymousContainingBlock(
LayoutObject* container,
AncestorSkipInfo* skip_info) {
// For inlines, we return the nearest non-anonymous enclosing
// block. We don't try to return the inline itself. This allows us to avoid
// having a positioned objects list in all LayoutInlines and lets us return a
// strongly-typed LayoutBlock* result from this method. The
// LayoutObject::Container() method can actually be used to obtain the inline
// directly.
if (container && !container->IsLayoutBlock())
container = container->ContainingBlock(skip_info);
while (container && container->IsAnonymousBlock())
container = container->ContainingBlock(skip_info);
return DynamicTo<LayoutBlock>(container);
}
LayoutBlock* LayoutObject::ContainingBlockForAbsolutePosition( LayoutBlock* LayoutObject::ContainingBlockForAbsolutePosition(
AncestorSkipInfo* skip_info) const { AncestorSkipInfo* skip_info) const {
auto* container = ContainerForAbsolutePosition(skip_info); auto* container = ContainerForAbsolutePosition(skip_info);
@@ -1289,6 +1271,24 @@ LayoutBlock* LayoutObject::ContainingBlock(AncestorSkipInfo* skip_info) const {
return DynamicTo<LayoutBlock>(object); return DynamicTo<LayoutBlock>(object);
} }
LayoutBlock* LayoutObject::FindNonAnonymousContainingBlock(
LayoutObject* container,
AncestorSkipInfo* skip_info) {
// For inlines, we return the nearest non-anonymous enclosing
// block. We don't try to return the inline itself. This allows us to avoid
// having a positioned objects list in all LayoutInlines and lets us return a
// strongly-typed LayoutBlock* result from this method. The
// LayoutObject::Container() method can actually be used to obtain the inline
// directly.
if (container && !container->IsLayoutBlock())
container = container->ContainingBlock(skip_info);
while (container && container->IsAnonymousBlock())
container = container->ContainingBlock(skip_info);
return DynamicTo<LayoutBlock>(container);
}
bool LayoutObject::ComputeIsFixedContainer(const ComputedStyle* style) const { bool LayoutObject::ComputeIsFixedContainer(const ComputedStyle* style) const {
if (!style) if (!style)
return false; return false;

9
third_party/blink/renderer/core/layout/layout_object.h vendored

@@ -1588,6 +1588,11 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
// See LayoutBlock.h for some extra explanations on containing blocks. // See LayoutBlock.h for some extra explanations on containing blocks.
LayoutBlock* ContainingBlock(AncestorSkipInfo* = nullptr) const; LayoutBlock* ContainingBlock(AncestorSkipInfo* = nullptr) const;
// Returns |container|'s containing block.
static LayoutBlock* FindNonAnonymousContainingBlock(
LayoutObject* container,
AncestorSkipInfo* = nullptr);
const LayoutBlock* InclusiveContainingBlock() const; const LayoutBlock* InclusiveContainingBlock() const;
bool CanContainAbsolutePositionObjects() const { bool CanContainAbsolutePositionObjects() const {
@@ -2718,10 +2723,6 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
bitfields_.SetBackgroundIsKnownToBeObscured(b); bitfields_.SetBackgroundIsKnownToBeObscured(b);
} }
// Returns |container|'s containing block.
static LayoutBlock* FindNonAnonymousContainingBlock(
LayoutObject* container,
AncestorSkipInfo* = nullptr);
// Returns ContainerForAbsolutePosition() if it's a LayoutBlock, or the // Returns ContainerForAbsolutePosition() if it's a LayoutBlock, or the
// containing LayoutBlock of it. // containing LayoutBlock of it.
LayoutBlock* ContainingBlockForAbsolutePosition( LayoutBlock* ContainingBlockForAbsolutePosition(

8
third_party/blink/renderer/modules/accessibility/ax_layout_object.cc vendored

@@ -267,8 +267,12 @@ Node* AXLayoutObject::GetNodeOrContainingBlockNode() const {
if (layout_object_->IsListMarker()) if (layout_object_->IsListMarker())
return ToLayoutListMarker(layout_object_)->ListItem()->GetNode(); return ToLayoutListMarker(layout_object_)->ListItem()->GetNode();
if (layout_object_->IsAnonymous() && layout_object_->ContainingBlock()) { if (layout_object_->IsAnonymous()) {
return layout_object_->ContainingBlock()->GetNode(); if (LayoutBlock* layout_block =
LayoutObject::FindNonAnonymousContainingBlock(layout_object_)) {
return layout_block->GetNode();
}
return nullptr;
} }
return GetNode(); return GetNode();

8
ui/accessibility/platform/ax_platform_node_textrangeprovider_win_unittest.cc

@@ -3636,10 +3636,16 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
ui::AXNodeData link_data; ui::AXNodeData link_data;
link_data.id = 16; link_data.id = 16;
link_data.role = ax::mojom::Role::kLink; link_data.role = ax::mojom::Role::kLink;
link_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
link_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
ui::AXNodeData link_text_data; ui::AXNodeData link_text_data;
link_text_data.id = 17; link_text_data.id = 17;
link_text_data.role = ax::mojom::Role::kStaticText; link_text_data.role = ax::mojom::Role::kStaticText;
link_text_data.AddIntAttribute(ax::mojom::IntAttribute::kBackgroundColor,
0xDEADBEEFU);
link_text_data.AddIntAttribute(ax::mojom::IntAttribute::kColor, 0xDEADC0DEU);
link_data.child_ids = {17}; link_data.child_ids = {17};
ui::AXNodeData root_data; ui::AXNodeData root_data;
@@ -3726,6 +3732,8 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
expected_variant.Set(static_cast<int32_t>(0x00EFBEADU)); expected_variant.Set(static_cast<int32_t>(0x00EFBEADU));
EXPECT_UIA_TEXTATTRIBUTE_EQ(text_range_provider, EXPECT_UIA_TEXTATTRIBUTE_EQ(text_range_provider,
UIA_BackgroundColorAttributeId, expected_variant); UIA_BackgroundColorAttributeId, expected_variant);
// Important: all nodes need to have the kColor and kBackgroundColor attribute
// set for this test, otherwise the following assert will fail.
EXPECT_UIA_TEXTATTRIBUTE_EQ(document_range_provider, EXPECT_UIA_TEXTATTRIBUTE_EQ(document_range_provider,
UIA_BackgroundColorAttributeId, expected_variant); UIA_BackgroundColorAttributeId, expected_variant);
expected_variant.Reset(); expected_variant.Reset();