Commit d543bbc7 authored by Ankit Kumar's avatar Ankit Kumar Committed by Commit Bot

Reland "Add contents of popup note associated with a highlight in PDF accessibility tree"

This is a reland of 9c48a1c1

Original CL: http://crrev.com/c/2208832
Revert CL: http://crrev.com/c/2247941

Reason for revert:
Causing PDFExtensionAccessiblity tests to fail on Mac Debug.

The method GetStaticTextNodeFromNode() had an assumption that highlight
node would only have one children. The original CL changed that as the
popup note is added as a child of the highlight node.

Original change's description:
> Add contents of popup note associated with a highlight in PDF accessibility tree
>
> This CL adds the note text data associated with a highlight to the
> accessibility tree. The CL makes the content of the note text accessible
> to users via screen readers.
>
> Tests have been modified to validate the addition of note text data to
> the accessibility tree.
>
> Bug: 1008775
> Change-Id: I086d039b9296d0b8ac1b13926835375a7943f8c9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208832
> Reviewed-by: Kevin Babbitt <kbabbitt@microsoft.com>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Commit-Queue: Ankit Kumar 🌪️ <ankk@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#779008}

CQ_INCLUDE_TRYBOTS=luci.chromium.try:mac_chromium_dbg_ng

Bug: 1008775
Change-Id: If9eb9ae1a0f975b99e00e01ca68e6d301989cd93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2249263Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Ankit Kumar 🌪️ <ankk@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#779572}
parent c7a75fa6
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
++++++[paragraph] ++++++[paragraph]
++++++++[push button] name='Hello' ++++++++[push button] name='Hello'
++++++++++[static] name='Hello' ++++++++++[static] name='Hello'
++++++++++[comment] name='Text Note'
++++++++[static] name=', nice ' ++++++++[static] name=', nice '
++++++++[push button] name='meeting' ++++++++[push button] name='meeting'
++++++++++[static] name='meeting' ++++++++++[static] name='meeting'
......
...@@ -5,6 +5,7 @@ embeddedObject ...@@ -5,6 +5,7 @@ embeddedObject
++++++++pdfActionableHighlight name='Hello' roleDescription='Highlight' restriction=readOnly ++++++++pdfActionableHighlight name='Hello' roleDescription='Highlight' restriction=readOnly
++++++++++staticText name='Hello' restriction=readOnly ++++++++++staticText name='Hello' restriction=readOnly
++++++++++++inlineTextBox name='Hello' restriction=readOnly ++++++++++++inlineTextBox name='Hello' restriction=readOnly
++++++++++note name='Text Note' roleDescription='Note' restriction=readOnly
++++++++staticText name=', nice ' restriction=readOnly ++++++++staticText name=', nice ' restriction=readOnly
++++++++++inlineTextBox name=', nice ' restriction=readOnly ++++++++++inlineTextBox name=', nice ' restriction=readOnly
++++++++pdfActionableHighlight name='meeting' roleDescription='Highlight' restriction=readOnly ++++++++pdfActionableHighlight name='meeting' roleDescription='Highlight' restriction=readOnly
......
...@@ -4,6 +4,7 @@ AXGroup AXDescription='PDF document containing 1 page' ...@@ -4,6 +4,7 @@ AXGroup AXDescription='PDF document containing 1 page'
++++++AXGroup ++++++AXGroup
++++++++AXButton AXDescription='Hello' ++++++++AXButton AXDescription='Hello'
++++++++++AXStaticText AXValue='Hello' ++++++++++AXStaticText AXValue='Hello'
++++++++++AXGroup AXDescription='Text Note'
++++++++AXStaticText AXValue=', nice ' ++++++++AXStaticText AXValue=', nice '
++++++++AXButton AXDescription='meeting' ++++++++AXButton AXDescription='meeting'
++++++++++AXStaticText AXValue='meeting' ++++++++++AXStaticText AXValue='meeting'
......
...@@ -4,6 +4,7 @@ Pane ...@@ -4,6 +4,7 @@ Pane
++++++Group IsControlElement=false ++++++Group IsControlElement=false
++++++++Custom Name='Hello' ++++++++Custom Name='Hello'
++++++++++Text Name='Hello' IsControlElement=false ++++++++++Text Name='Hello' IsControlElement=false
++++++++++Group Name='Text Note'
++++++++Text Name=', nice ' ++++++++Text Name=', nice '
++++++++Custom Name='meeting' ++++++++Custom Name='meeting'
++++++++++Text Name='meeting' IsControlElement=false ++++++++++Text Name='meeting' IsControlElement=false
......
...@@ -4,6 +4,7 @@ IA2_ROLE_SECTION UNAVAILABLE FOCUSABLE ...@@ -4,6 +4,7 @@ IA2_ROLE_SECTION UNAVAILABLE FOCUSABLE
++++++IA2_ROLE_PARAGRAPH READONLY ++++++IA2_ROLE_PARAGRAPH READONLY
++++++++ROLE_SYSTEM_PUSHBUTTON name='Hello' READONLY ++++++++ROLE_SYSTEM_PUSHBUTTON name='Hello' READONLY
++++++++++ROLE_SYSTEM_STATICTEXT name='Hello' READONLY ++++++++++ROLE_SYSTEM_STATICTEXT name='Hello' READONLY
++++++++++IA2_ROLE_NOTE name='Text Note' READONLY
++++++++ROLE_SYSTEM_STATICTEXT name=', nice ' READONLY ++++++++ROLE_SYSTEM_STATICTEXT name=', nice ' READONLY
++++++++ROLE_SYSTEM_PUSHBUTTON name='meeting' READONLY ++++++++ROLE_SYSTEM_PUSHBUTTON name='meeting' READONLY
++++++++++ROLE_SYSTEM_STATICTEXT name='meeting' READONLY ++++++++++ROLE_SYSTEM_STATICTEXT name='meeting' READONLY
......
...@@ -20,7 +20,7 @@ endobj ...@@ -20,7 +20,7 @@ endobj
>> >>
>> >>
/Contents 5 0 R /Contents 5 0 R
/Annots [6 0 R 7 0 R 8 0 R] /Annots [6 0 R 7 0 R 8 0 R 9 0 R]
>> >>
endobj endobj
{{object 4 0}} << {{object 4 0}} <<
...@@ -43,6 +43,8 @@ endobj ...@@ -43,6 +43,8 @@ endobj
{{object 6 0}} << {{object 6 0}} <<
/Type /Annot /Type /Annot
/Subtype /Highlight /Subtype /Highlight
/Popup 9 0 R
/Contents (Text Note)
/QuadPoints [0 55 36 59 0 36 36 36] /QuadPoints [0 55 36 59 0 36 36 36]
/Rect [0 36 36 55] /Rect [0 36 36 55]
/P 3 0 R /P 3 0 R
...@@ -64,6 +66,13 @@ endobj ...@@ -64,6 +66,13 @@ endobj
/P 3 0 R /P 3 0 R
>> >>
endobj endobj
{{object 9 0}} <<
/Type /Annot
/Subtype /Popup
/Parent 6 0 R
/Rect [140 36 149 55]
>>
endobj
{{xref}} {{xref}}
{{trailer}} {{trailer}}
{{startxref}} {{startxref}}
......
This diff was suppressed by a .gitattributes entry.
...@@ -230,14 +230,19 @@ bool BreakParagraph( ...@@ -230,14 +230,19 @@ bool BreakParagraph(
ui::AXNode* GetStaticTextNodeFromNode(ui::AXNode* node) { ui::AXNode* GetStaticTextNodeFromNode(ui::AXNode* node) {
// Returns the appropriate static text node given |node|'s type. // Returns the appropriate static text node given |node|'s type.
// Returns nullptr if there is no appropriate static text node. // Returns nullptr if there is no appropriate static text node.
if (!node)
return nullptr;
ui::AXNode* static_node = node; ui::AXNode* static_node = node;
// Get the static text from the link node. // Get the static text from the link node.
if (node && if (node->data().role == ax::mojom::Role::kLink &&
(node->data().role == ax::mojom::Role::kLink ||
node->data().role == ax::mojom::Role::kPdfActionableHighlight) &&
node->children().size() == 1) { node->children().size() == 1) {
static_node = node->children()[0]; static_node = node->children()[0];
} }
// Get the static text from the highlight node.
if (node->data().role == ax::mojom::Role::kPdfActionableHighlight &&
!node->children().empty()) {
static_node = node->children()[0];
}
// If it's static text node, then it holds text. // If it's static text node, then it holds text.
if (static_node && static_node->data().role == ax::mojom::Role::kStaticText) if (static_node && static_node->data().role == ax::mojom::Role::kStaticText)
return static_node; return static_node;
...@@ -649,6 +654,23 @@ class PdfAccessibilityTreeBuilder { ...@@ -649,6 +654,23 @@ class PdfAccessibilityTreeBuilder {
return highlight_node; return highlight_node;
} }
ui::AXNodeData* CreatePopupNoteNode(
const ppapi::PdfAccessibilityHighlightInfo& highlight) {
ui::AXNodeData* popup_note_node =
CreateNode(ax::mojom::Role::kNote, ax::mojom::Restriction::kReadOnly,
render_accessibility_, nodes_);
popup_note_node->AddStringAttribute(
ax::mojom::StringAttribute::kRoleDescription,
l10n_util::GetStringUTF8(IDS_AX_ROLE_DESCRIPTION_PDF_POPUP_NOTE));
popup_note_node->AddStringAttribute(ax::mojom::StringAttribute::kName,
highlight.note_text);
popup_note_node->relative_bounds.bounds =
PpFloatRectToGfxRectF(highlight.bounds);
return popup_note_node;
}
ui::AXNodeData* CreateTextFieldNode( ui::AXNodeData* CreateTextFieldNode(
const ppapi::PdfAccessibilityTextFieldInfo& text_field) { const ppapi::PdfAccessibilityTextFieldInfo& text_field) {
ax::mojom::Restriction restriction = text_field.is_read_only ax::mojom::Restriction restriction = text_field.is_read_only
...@@ -811,6 +833,11 @@ class PdfAccessibilityTreeBuilder { ...@@ -811,6 +833,11 @@ class PdfAccessibilityTreeBuilder {
AddTextToObjectNode(highlight.text_run_index, highlight.text_run_count, AddTextToObjectNode(highlight.text_run_index, highlight.text_run_count,
highlight_node, para_node, previous_on_line_node, highlight_node, para_node, previous_on_line_node,
text_run_index); text_run_index);
if (!highlight.note_text.empty()) {
ui::AXNodeData* popup_note_node = CreatePopupNoteNode(highlight);
highlight_node->child_ids.push_back(popup_note_node->id);
}
} }
void AddTextFieldToParaNode( void AddTextFieldToParaNode(
......
...@@ -448,6 +448,7 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) { ...@@ -448,6 +448,7 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) {
chrome_pdf::features::kAccessiblePDFHighlight); chrome_pdf::features::kAccessiblePDFHighlight);
constexpr uint32_t kHighlightWhiteColor = MakeARGB(255, 255, 255, 255); constexpr uint32_t kHighlightWhiteColor = MakeARGB(255, 255, 255, 255);
const char kPopupNoteText[] = "Text Note";
text_runs_.emplace_back(kFirstTextRun); text_runs_.emplace_back(kFirstTextRun);
text_runs_.emplace_back(kSecondTextRun); text_runs_.emplace_back(kSecondTextRun);
...@@ -460,6 +461,7 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) { ...@@ -460,6 +461,7 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) {
highlight.text_run_index = 0; highlight.text_run_index = 0;
highlight.text_run_count = 2; highlight.text_run_count = 2;
highlight.color = kHighlightWhiteColor; highlight.color = kHighlightWhiteColor;
highlight.note_text = kPopupNoteText;
page_objects_.highlights.push_back(std::move(highlight)); page_objects_.highlights.push_back(std::move(highlight));
} }
...@@ -486,6 +488,7 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) { ...@@ -486,6 +488,7 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) {
* ++++ Paragraph * ++++ Paragraph
* ++++++ Highlight * ++++++ Highlight
* ++++++++ Static Text * ++++++++ Static Text
* ++++++++ Note
*/ */
ui::AXNode* root_node = pdf_accessibility_tree.GetRoot(); ui::AXNode* root_node = pdf_accessibility_tree.GetRoot();
...@@ -517,12 +520,23 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) { ...@@ -517,12 +520,23 @@ TEST_F(PdfAccessibilityTreeTest, TestHighlightCreation) {
EXPECT_EQ(kHighlightWhiteColor, EXPECT_EQ(kHighlightWhiteColor,
static_cast<uint32_t>(highlight_node->GetIntAttribute( static_cast<uint32_t>(highlight_node->GetIntAttribute(
ax::mojom::IntAttribute::kBackgroundColor))); ax::mojom::IntAttribute::kBackgroundColor)));
ASSERT_EQ(1u, highlight_node->children().size()); ASSERT_EQ(2u, highlight_node->children().size());
ui::AXNode* static_text_node = highlight_node->children()[0]; ui::AXNode* static_text_node = highlight_node->children()[0];
ASSERT_TRUE(static_text_node); ASSERT_TRUE(static_text_node);
EXPECT_EQ(ax::mojom::Role::kStaticText, static_text_node->data().role); EXPECT_EQ(ax::mojom::Role::kStaticText, static_text_node->data().role);
ASSERT_EQ(2u, static_text_node->children().size()); ASSERT_EQ(2u, static_text_node->children().size());
ui::AXNode* popup_note_node = highlight_node->children()[1];
ASSERT_TRUE(popup_note_node);
EXPECT_EQ(ax::mojom::Role::kNote, popup_note_node->data().role);
EXPECT_EQ(kPopupNoteText, popup_note_node->GetStringAttribute(
ax::mojom::StringAttribute::kName));
EXPECT_EQ(l10n_util::GetStringUTF8(IDS_AX_ROLE_DESCRIPTION_PDF_POPUP_NOTE),
popup_note_node->GetStringAttribute(
ax::mojom::StringAttribute::kRoleDescription));
EXPECT_EQ(gfx::RectF(1.0f, 1.0f, 5.0f, 6.0f),
popup_note_node->data().relative_bounds.bounds);
} }
TEST_F(PdfAccessibilityTreeTest, TestTextFieldNodeCreation) { TEST_F(PdfAccessibilityTreeTest, TestTextFieldNodeCreation) {
......
...@@ -242,5 +242,8 @@ ...@@ -242,5 +242,8 @@
<message name="IDS_AX_ROLE_DESCRIPTION_PDF_HIGHLIGHT" desc="Accessibility role description for PDF highlight"> <message name="IDS_AX_ROLE_DESCRIPTION_PDF_HIGHLIGHT" desc="Accessibility role description for PDF highlight">
Highlight Highlight
</message> </message>
<message name="IDS_AX_ROLE_DESCRIPTION_PDF_POPUP_NOTE" desc="Accessibility role description for popup note annotation associated with a PDF highlight">
Note
</message>
</if> </if>
</grit-part> </grit-part>
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