More precise use of multiline state

content/browser/accessibility/browser_accessibility_com_win.cc

Index: content/browser/accessibility/browser_accessibility_com_win.cc
diff --git a/content/browser/accessibility/browser_accessibility_com_win.cc b/content/browser/accessibility/browser_accessibility_com_win.cc
index 8ec9e981b9f157a48844fd71d957dab25cbeaf93..c415c6b96e5b9a3f71b873f34c53f5e2bca57a4c 100644
--- a/content/browser/accessibility/browser_accessibility_com_win.cc
+++ b/content/browser/accessibility/browser_accessibility_com_win.cc
@@ -2932,8 +2932,7 @@ STDMETHODIMP BrowserAccessibilityComWin::get_nodeInfo(
   *num_children = owner()->PlatformChildCount();
   *unique_id = -owner()->unique_id();
 
-  if (owner()->GetRole() == ui::AX_ROLE_ROOT_WEB_AREA ||
-      owner()->GetRole() == ui::AX_ROLE_WEB_AREA) {
+  if (IsDocument()) {
     *node_type = NODETYPE_DOCUMENT;
   } else if (owner()->IsTextOnlyObject()) {
     *node_type = NODETYPE_TEXT;
@@ -3777,8 +3776,7 @@ void BrowserAccessibilityComWin::UpdateStep1ComputeWinAttributes() {
     win_attributes_->ia2_attributes.push_back(L"valuetext:" + value);
   } else {
     // On Windows, the value of a document should be its url.
-    if (owner()->GetRole() == ui::AX_ROLE_ROOT_WEB_AREA ||
-        owner()->GetRole() == ui::AX_ROLE_WEB_AREA) {
+    if (IsDocument()) {
       value = base::UTF8ToUTF16(Manager()->GetTreeData().url);
     }
     // If this doesn't have a value and is linked then set its value to the url
@@ -4302,6 +4300,27 @@ void BrowserAccessibilityComWin::IntAttributeToIA2(ui::AXIntAttribute attribute,
   }
 }
 
+bool BrowserAccessibilityComWin::IsDocument() const {
+  return owner()->GetRole() == ui::AX_ROLE_ROOT_WEB_AREA ||
+         owner()->GetRole() == ui::AX_ROLE_WEB_AREA;
+}
+
+bool BrowserAccessibilityComWin::IsRootEditable() const {
+  if (!owner()->HasState(ui::AX_STATE_EDITABLE))
+    return false;
+
+  if (IsDocument())
+    return true;
+
+  BrowserAccessibility* parent = owner()->PlatformGetParent();

[dougt, 2017/06/23 18:27:18]

I am confused by this. 

GetParent() is just returning the node above |this| one.  Shouldn't we walk up
the tree to the root?

[aleventhal, 2017/06/28 14:20:57]

It does what I want, but maybe it needs a different name.

This is looking to see if we're in an editable area AND we're in the top
editable element of that area. If the thing above us is not editable, then we
aren't in a root editable.

For example:

Section -- not editable
  Paragraph -- editable
    Link -- editable

Only the paragraph there is a root editable.

+  return !parent || !parent->HasState(ui::AX_STATE_EDITABLE);
+}
+
+bool BrowserAccessibilityComWin::IsEditBoxRole() const {
+  const int32_t role = owner()->GetRole();
+  return role == ui::AX_ROLE_TEXT_FIELD || role == ui::AX_ROLE_SEARCH_BOX;
+}
+
 bool BrowserAccessibilityComWin::IsHyperlink() const {
   int32_t hyperlink_index = -1;
   auto* parent = owner()->PlatformGetParent();
@@ -4363,7 +4382,7 @@ int32_t BrowserAccessibilityComWin::GetHypertextOffsetFromChild(
   // Handle the case when we are dealing with a direct text-only child.
   // (Note that this object might be a platform leaf, e.g. an ARIA searchbox,
   // and so |owner()->InternalChild...| functions need to be used. Also,
-  // direct text-only children should not be present at tree roots and so no
+  // direct text-only children should not be present at tree  s and so no

[dougt, 2017/06/23 18:27:18]

I don't think you want this change.

[aleventhal, 2017/06/28 14:20:57]

Done.

   // cross-tree traversal is necessary.)
   if (child.owner()->IsTextOnlyObject()) {
     int32_t hypertextOffset = 0;
@@ -5078,8 +5097,22 @@ void BrowserAccessibilityComWin::InitRoleAndState() {
       ia_state |= STATE_SYSTEM_HOTTRACKED;
   }
 
-  if (owner()->HasState(ui::AX_STATE_EDITABLE))
+  const bool is_editable = owner()->HasState(ui::AX_STATE_EDITABLE);
+  if (is_editable) {
     ia2_state |= IA2_STATE_EDITABLE;
+  }

[dougt, 2017/06/23 18:27:17]

Since this is a one liner, you don't need the curly braces.

[aleventhal, 2017/06/28 14:20:57]

Done.

+
+  if (is_editable ? IsRootEditable() : IsEditBoxRole()) {
+    // Support multi/single line states if root edtiable or appropriate role.
+    // We support the edit box roles even if the area is not actually editable,
+    // because it is technically feasible for JS to implement the edit box
+    // by controlling selection.
+    if (owner()->HasState(ui::AX_STATE_MULTILINE)) {
+      ia2_state |= IA2_STATE_MULTI_LINE;
+    } else {
+      ia2_state |= IA2_STATE_SINGLE_LINE;
+    }
+  }
 
   if (!owner()->GetStringAttribute(ui::AX_ATTR_AUTO_COMPLETE).empty())
     ia2_state |= IA2_STATE_SUPPORTS_AUTOCOMPLETION;
@@ -5477,11 +5510,6 @@ void BrowserAccessibilityComWin::InitRoleAndState() {
     case ui::AX_ROLE_TEXT_FIELD:
     case ui::AX_ROLE_SEARCH_BOX:
       ia_role = ROLE_SYSTEM_TEXT;
-      if (owner()->HasState(ui::AX_STATE_MULTILINE)) {
-        ia2_state |= IA2_STATE_MULTI_LINE;
-      } else {
-        ia2_state |= IA2_STATE_SINGLE_LINE;
-      }
       if (owner()->HasState(ui::AX_STATE_READ_ONLY))
         ia_state |= STATE_SYSTEM_READONLY;
       ia2_state |= IA2_STATE_SELECTABLE_TEXT;