[Read Anything] Add text formatting for PDFs

Before this change, Reading mode received the text content for an
entire PDF from a single node (the embedded node) in the PDF
accessibility tree. This caused all the PDF content to be formatted as
paragraph text in Reading Mode.

Now, Reading mode will receive text content from the individual static
text nodes from the PDF tree. This allows the text formatting of
the original PDF to be preserved.

Bug: 1266555
Change-Id: Iff7ab7a267794f7707d5d2c59be211eda56899e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5007030
Reviewed-by: Abigail Klein <abigailbklein@google.com>
Commit-Queue: Jocelyn Tran <jocelyntran@google.com>
Cr-Commit-Position: refs/heads/main@{#1225688}

chrome/browser/resources/side_panel/read_anything/app.html

@@ -39,6 +39,7 @@
     letter-spacing: var(--letter-spacing);
     line-height: var(--line-height);
     padding: 20px;
+    /* max-width should match line width limit in read_anything_constants.h. */
     max-width: 60ch;
     margin: 0 auto;
   }

chrome/common/accessibility/read_anything_constants.h

@@ -50,6 +50,10 @@ const int kFontSizeIconSize = kIconSize + kInternalInsets;
 const int kColorsIconSize = 24;
 const int kSpacingIconSize = 20;
 
+// Used for text formatting correction in PDFs. This value should match the line
+// width limit in app.html.
+const int kMaxLineWidth = 60;
+
 // Audio constants for Read Aloud feature.
 // Speech rate is a multiplicative scale where 1 is the baseline.
 const double kReadAnythingDefaultSpeechRate = 1;

chrome/renderer/accessibility/read_anything_app_controller.cc

@@ -890,27 +890,26 @@ std::string ReadAnythingAppController::GetHtmlTag(
   ui::AXNode* ax_node = model_.GetAXNode(ax_node_id);
   DCHECK(ax_node);
 
+  std::string html_tag =
+      ax_node->GetStringAttribute(ax::mojom::StringAttribute::kHtmlTag);
+
+  if (model_.is_pdf()) {
+    return GetHtmlTagForPDF(ax_node, html_tag);
+  }
+
   if (ui::IsTextField(ax_node->GetRole())) {
     return "div";
   }
 
   // Some divs are marked with role=heading and aria-level=# to indicate
   // the heading level, so use the <h#> tag directly.
   if (ax_node->GetRole() == ax::mojom::Role::kHeading) {
-    std::string aria_level;
-    ax_node->GetHtmlAttribute("aria-level", &aria_level);
+    std::string aria_level = GetAriaLevel(ax_node);
     if (!aria_level.empty()) {
       return "h" + aria_level;
     }
   }
 
-  // Replace embedded objects with div to display PDF content.
-  if (ax_node->GetRole() == ax::mojom::Role::kEmbeddedObject) {
-    return "div";
-  }
-
-  std::string html_tag =
-      ax_node->GetStringAttribute(ax::mojom::StringAttribute::kHtmlTag);
   if (html_tag == ui::ToString(ax::mojom::Role::kMark)) {
     // Replace mark element with bold element for readability.
     html_tag = "b";
@@ -928,6 +927,65 @@ std::string ReadAnythingAppController::GetHtmlTag(
   return html_tag;
 }
 
+std::string ReadAnythingAppController::GetAriaLevel(ui::AXNode* ax_node) const {
+  std::string aria_level;
+  ax_node->GetHtmlAttribute("aria-level", &aria_level);
+  return aria_level;
+}
+
+std::string ReadAnythingAppController::GetHtmlTagForPDF(
+    ui::AXNode* ax_node,
+    std::string html_tag) const {
+  ax::mojom::Role role = ax_node->GetRole();
+
+  // Some nodes in PDFs don't have an HTML tag so use role instead.
+  switch (role) {
+    case ax::mojom::Role::kEmbeddedObject:
+    case ax::mojom::Role::kRegion:
+    case ax::mojom::Role::kPdfRoot:
+    case ax::mojom::Role::kRootWebArea:
+      return "span";
+    case ax::mojom::Role::kParagraph:
+      return "p";
+    case ax::mojom::Role::kLink:
+      return "a";
+    case ax::mojom::Role::kStaticText:
+      return "";
+    case ax::mojom::Role::kHeading:
+      return GetHeadingHtmlTagForPDF(ax_node, html_tag);
+    default:
+      return html_tag;
+  }
+}
+
+std::string ReadAnythingAppController::GetHeadingHtmlTagForPDF(
+    ui::AXNode* ax_node,
+    std::string html_tag) const {
+  // Sometimes whole paragraphs can be formatted as a heading. If the text is
+  // longer than 2 lines, assume it was meant to be a paragragh,
+  if (ax_node->GetTextContentUTF8().length() > (2 * kMaxLineWidth)) {
+    return "p";
+  }
+
+  // A single block of text could be incorrectly formatted with multiple heading
+  // nodes (one for each line of text) instead of a single paragraph node. This
+  // case should be detected to improve readability. If there are multiple
+  // consecutive nodes with the same heading level, assume that they are all a
+  // part of one paragraph.
+  ui::AXNode* next = ax_node->GetNextUnignoredSibling();
+  ui::AXNode* prev = ax_node->GetPreviousUnignoredSibling();
+
+  if ((next && next->GetStringAttribute(ax::mojom::StringAttribute::kHtmlTag) ==
+                   html_tag) ||
+      (prev && prev->GetStringAttribute(ax::mojom::StringAttribute::kHtmlTag) ==
+                   html_tag)) {
+    return "span";
+  }
+
+  std::string aria_level = GetAriaLevel(ax_node);
+  return !aria_level.empty() ? "h" + aria_level : html_tag;
+}
+
 std::string ReadAnythingAppController::GetLanguage(
     ui::AXNodeID ax_node_id) const {
   ui::AXNode* ax_node = model_.GetAXNode(ax_node_id);

chrome/renderer/accessibility/read_anything_app_controller.h

@@ -182,6 +182,11 @@ class ReadAnythingAppController
   double GetLetterSpacingValue(int letter_spacing) const;
   std::vector<std::string> GetSupportedFonts() const;
 
+  std::string GetHtmlTagForPDF(ui::AXNode* ax_node, std::string html_tag) const;
+  std::string GetHeadingHtmlTagForPDF(ui::AXNode* ax_node,
+                                      std::string html_tag) const;
+  std::string GetAriaLevel(ui::AXNode* ax_node) const;
+
   // The language code that should be used to determine which voices are
   // supported for speech.
   const std::string& GetLanguageCodeForSpeech() const;

chrome/renderer/accessibility/read_anything_app_controller_browsertest.cc

@@ -137,6 +137,35 @@ class ReadAnythingAppControllerTest : public ChromeRenderViewTest {
     Mock::VerifyAndClearExpectations(distiller_);
   }
 
+  ui::AXTreeID SetUpPdfTrees() {
+    // Call OnActiveAXTreeIDChanged() to set is_pdf_ state.
+    GURL pdf_url("http://www.google.com/foo/bar.pdf");
+    OnActiveAXTreeIDChanged(tree_id_, pdf_url, true);
+
+    // PDF set up required for formatting checks.
+    ui::AXTreeID pdf_iframe_tree_id = ui::AXTreeID::CreateNewAXTreeID();
+    ui::AXTreeID pdf_web_contents_tree_id = ui::AXTreeID::CreateNewAXTreeID();
+
+    // Send update for main web content with child tree (pdf web contents).
+    ui::AXTreeUpdate main_web_contents_update;
+    SetUpdateTreeID(&main_web_contents_update);
+    main_web_contents_update.nodes.resize(1);
+    main_web_contents_update.nodes[0].id = 1;
+    main_web_contents_update.nodes[0].AddChildTreeId(pdf_web_contents_tree_id);
+    AccessibilityEventReceived({main_web_contents_update});
+
+    // Send update for pdf web contents with child tree (iframe).
+    ui::AXTreeUpdate pdf_web_contents_update;
+    pdf_web_contents_update.nodes.resize(1);
+    pdf_web_contents_update.root_id = 1;
+    pdf_web_contents_update.nodes[0].id = 1;
+    pdf_web_contents_update.nodes[0].AddChildTreeId(pdf_iframe_tree_id);
+    SetUpdateTreeID(&pdf_web_contents_update, pdf_web_contents_tree_id);
+    AccessibilityEventReceived({pdf_web_contents_update});
+
+    return pdf_iframe_tree_id;
+  }
+
   void SetUpdateTreeID(ui::AXTreeUpdate* update) {
     SetUpdateTreeID(update, tree_id_);
   }
@@ -545,6 +574,73 @@ TEST_F(ReadAnythingAppControllerTest,
   EXPECT_EQ(h3, GetHtmlTag(3));
 }
 
+TEST_F(ReadAnythingAppControllerTest, GetHtmlTag_PDF) {
+  ui::AXTreeID pdf_iframe_tree_id = SetUpPdfTrees();
+
+  // Send pdf iframe update with html tags to test.
+  ui::AXTreeUpdate update;
+  SetUpdateTreeID(&update, pdf_iframe_tree_id);
+  update.nodes.resize(3);
+  update.root_id = 1;
+  update.nodes[0].id = 1;
+  update.nodes[0].child_ids = {2, 3};
+  update.nodes[1].id = 2;
+  update.nodes[2].id = 3;
+  update.nodes[0].role = ax::mojom::Role::kPdfRoot;
+  update.nodes[1].AddStringAttribute(ax::mojom::StringAttribute::kHtmlTag,
+                                     "h1");
+  update.nodes[2].role = ax::mojom::Role::kHeading;
+  update.nodes[2].html_attributes.emplace_back("aria-level", "2");
+  AccessibilityEventReceived({update});
+
+  OnAXTreeDistilled({});
+  EXPECT_CALL(page_handler_, EnablePDFContentAccessibility).Times(1);
+  EXPECT_EQ("span", GetHtmlTag(1));
+  EXPECT_EQ("h1", GetHtmlTag(2));
+  EXPECT_EQ("h2", GetHtmlTag(3));
+}
+
+TEST_F(ReadAnythingAppControllerTest, GetHtmlTag_IncorrectlyFormattedPDF) {
+  ui::AXTreeID pdf_iframe_tree_id = SetUpPdfTrees();
+
+  // Send pdf iframe update with html tags to test. Two headings next to each
+  // other should be spans. A heading that's too long should be turned into a
+  // paragraph.
+  ui::AXTreeUpdate update;
+  SetUpdateTreeID(&update, pdf_iframe_tree_id);
+  update.nodes.resize(5);
+  update.root_id = 1;
+  update.nodes[0].id = 1;
+  update.nodes[0].child_ids = {2, 3, 4, 5};
+  update.nodes[1].id = 2;
+  update.nodes[2].id = 3;
+  update.nodes[3].id = 4;
+  update.nodes[4].id = 5;
+  update.nodes[0].role = ax::mojom::Role::kPdfRoot;
+  update.nodes[1].role = ax::mojom::Role::kHeading;
+  update.nodes[1].AddStringAttribute(ax::mojom::StringAttribute::kHtmlTag,
+                                     "h1");
+  update.nodes[2].role = ax::mojom::Role::kHeading;
+  update.nodes[2].AddStringAttribute(ax::mojom::StringAttribute::kHtmlTag,
+                                     "h1");
+  update.nodes[3].role = ax::mojom::Role::kLink;
+  update.nodes[4].role = ax::mojom::Role::kHeading;
+  update.nodes[4].html_attributes.emplace_back("aria-level", "1");
+  update.nodes[4].SetName(
+      "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod "
+      "tempor incididunt ut labore et dolore magna aliqua.");
+  update.nodes[4].SetNameFrom(ax::mojom::NameFrom::kContents);
+
+  AccessibilityEventReceived({update});
+
+  OnAXTreeDistilled({});
+  EXPECT_CALL(page_handler_, EnablePDFContentAccessibility).Times(1);
+  EXPECT_EQ("span", GetHtmlTag(2));
+  EXPECT_EQ("span", GetHtmlTag(3));
+  EXPECT_EQ("a", GetHtmlTag(4));
+  EXPECT_EQ("p", GetHtmlTag(5));
+}
+
 TEST_F(ReadAnythingAppControllerTest, GetTextContent_NoSelection) {
   std::string text_content = "Hello";
   std::string missing_text_content = "";