[Element Reflection] Null vs Empty-list differentiation.

If the content-attribute is not present, then we return null.

If the content attribute is present, and there are no explicitly set
attr-elements, and the content attribute doesn't yield any valid
elements, then we return an empty array.

This makes the interface more consistent, as a caller could already
to the attribute to null to clear.

This brings the implementation in-line with the update here
whatwg/html#3917 (comment)

This also updates/clarifies code comments and adds descriptive strings
to existing tests.

This is a subset of the work Alice did for https://crrev.com/c/2796854

Change-Id: I3dcc8492fdb5467f82468bbc3d4b7e4a87c5d48e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2825017
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Chris Hall <chrishall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#872329}
GitOrigin-RevId: 5bc9faabddc88997f080101649cd38a1c42875f8

blink/renderer/core/dom/document.h

@@ -246,8 +246,8 @@ enum ShadowCascadeOrder { kShadowCascadeNone, kShadowCascade };
 
 using DocumentClassFlags = unsigned char;
 
-// A map of IDL attribute name to Element value, for one particular element.
-// For example,
+// A map of IDL attribute name to Element list value, for one particular
+// element. For example,
 //   el1.ariaActiveDescendant = el2
 // would add the following pair to the ExplicitlySetAttrElementMap for el1:
 //   ("ariaActiveDescendant", el2)

blink/renderer/core/dom/element.cc

@@ -941,6 +941,9 @@ base::Optional<HeapVector<Member<Element>>> Element::GetElementArrayAttribute(
                : html_names::kAriaLabelledbyAttr;
   }
 
+  if (!hasAttribute(attr))
+    return base::nullopt;
+
   String attribute_value = getAttribute(attr).GetString();
   Vector<String> tokens;
   attribute_value = attribute_value.SimplifyWhiteSpace();
@@ -954,8 +957,6 @@ base::Optional<HeapVector<Member<Element>>> Element::GetElementArrayAttribute(
     if (candidate)
       result_elements.push_back(candidate);
   }
-  if (result_elements.IsEmpty())
-    return base::nullopt;
 
   return result_elements;
 }

blink/renderer/modules/accessibility/ax_relation_cache.cc

@@ -264,8 +264,8 @@ void AXRelationCache::UpdateAriaOwnsWithCleanLayout(AXObject* owner) {
   UpdateReverseRelations(owner, owned_id_vector);
 
   // We first check if the element has an explicitly set aria-owns association.
-  // Explicitly set elements are validated on setting time (that they are in a
-  // valid scope etc). The content attribute can contain ids that are not
+  // Explicitly set elements are validated when they are read (that they are in
+  // a valid scope etc). The content attribute can contain ids that are not
   // legally ownable.
   HeapVector<Member<AXObject>> owned_children;
   if (element && element->HasExplicitlySetAttrAssociatedElements(

blink/renderer/modules/accessibility/ax_sparse_attribute_setter.cc

@@ -70,7 +70,7 @@ void SetIntListAttribute(ax::mojom::blink::IntListAttribute attribute,
     return;
   base::Optional<HeapVector<Member<Element>>> attr_associated_elements =
       element->GetElementArrayAttribute(qualified_name);
-  if (!attr_associated_elements)
+  if (!attr_associated_elements || attr_associated_elements.value().IsEmpty())
     return;
   std::vector<int32_t> ax_ids;
 

blink/web_tests/accessibility/aria-owns-dynamic-changes.html

@@ -21,15 +21,15 @@
 
     child.id = "";
     assert_equals(axFutureParent.childrenCount, 0, "after clearing id");
-    assert_equals(futureParent.ariaOwnsElements, null);
+    assert_array_equals(futureParent.ariaOwnsElements, []);
 
     child.id = "future_child";
     assert_equals(axFutureParent.childrenCount, 1, "after setting id");
     assert_array_equals(futureParent.ariaOwnsElements, [child]);
 
     futureParent.setAttribute("aria-owns", "");
     assert_equals(axFutureParent.childrenCount, 0, "after clearing aria-owns");
-    assert_equals(futureParent.ariaOwnsElements, null);
+    assert_array_equals(futureParent.ariaOwnsElements, []);
 
     futureParent.setAttribute("aria-owns", "future_child");
     assert_equals(axFutureParent.childrenCount, 1, "after setting aria-oens");
@@ -46,7 +46,7 @@
 
     child.remove();
     assert_equals(axFutureParent.childrenCount, 0);
-    assert_equals(futureParent.ariaOwnsElements, null);
+    assert_array_equals(futureParent.ariaOwnsElements, []);
 }, "Aria-owns is dynamically recomputed.");
 </script>
 

blink/web_tests/external/wpt/dom/nodes/aria-element-reflection.tentative.html

@@ -147,7 +147,7 @@
   <script>
 
   test(function(t) {
-    assert_equals(passwordField.ariaDetailsElements, null);
+    assert_array_equals(passwordField.ariaDetailsElements, []);
     passwordField.ariaDetailsElements = [ listItem1 ];
     assert_equals(passwordField.getAttribute("aria-details"), "listItem1");
 
@@ -341,7 +341,7 @@
   test(function(t) {
     const billingElement = document.getElementById("billingElement")
     assert_array_equals(input1.ariaLabelledByElements, [billingElement, nameElement], "parsed content attribute sets element references.");
-    assert_equals(input2.ariaLabelledByElements, null, "Testing empty content attribute after parsing.");
+    assert_equals(input2.ariaLabelledByElements, null, "Testing missing content attribute after parsing.");
 
     input2.ariaLabelledByElements = [billingElement, addressElement];
     assert_array_equals(input2.ariaLabelledByElements, [billingElement, addressElement], "Testing IDL setter/getter.");
@@ -351,12 +351,12 @@
     // As it was explicitly set the underlying association will remain intact,
     // but it will be hidden until the element is moved back into a valid scope.
     billingElement.remove();
-    assert_array_equals(input2.ariaLabelledByElements, [addressElement]);
+    assert_array_equals(input2.ariaLabelledByElements, [addressElement], "Computed ariaLabelledByElements shouldn't include billing when out of scope.");
 
     // Insert the billingElement back into the DOM and check that it is visible
     // again, as the underlying association should have been kept intact.
     billingElementContainer.appendChild(billingElement);
-    assert_array_equals(input2.ariaLabelledByElements, [billingElement, addressElement]);
+    assert_array_equals(input2.ariaLabelledByElements, [billingElement, addressElement], "Billing element back in scope.");
 
     input2.ariaLabelledByElements = [];
     assert_array_equals(input2.ariaLabelledByElements, [], "Testing IDL setter/getter for empty array.");
@@ -366,7 +366,8 @@
     assert_equals(input1.ariaLabelledByElements, null);
 
     input1.setAttribute("aria-labelledby", "nameElement addressElement");
-    assert_array_equals(input1.ariaLabelledByElements, [nameElement, addressElement]);
+    assert_array_equals(input1.ariaLabelledByElements, [nameElement, addressElement],
+      "computed value after setting attribute directly");
 
     input1.ariaLabelledByElements = null;
     assert_false(input1.hasAttribute("aria-labelledby", "Nullifying the IDL attribute should remove the content attribute."));
@@ -516,7 +517,7 @@ <h2 id="lightDomHeading" aria-flowto="shadowChild1 shadowChild2">Light DOM Headi
 
     // The elements in the content attribute are in a "darker" tree - they
     // enter a shadow encapsulation boundary, so not be associated any more.
-    assert_equals(lightDomHeading.ariaFlowToElements, null);
+    assert_array_equals(lightDomHeading.ariaFlowToElements, []);
 
     // These elements are in a shadow including ancestor, i.e "lighter" tree.
     // Valid for the IDL attribute, but content attribute should be null.
@@ -526,7 +527,7 @@ <h2 id="lightDomHeading" aria-flowto="shadowChild1 shadowChild2">Light DOM Headi
     // These IDs belong to a different scope, so the attr-associated-element
     // cannot be computed.
     shadowChild2.setAttribute("aria-flowto", "lightDomText1 lightDomText2");
-    assert_equals(shadowChild2.ariaFlowToElements, null);
+    assert_array_equals(shadowChild2.ariaFlowToElements, []);
 
     // Elements that cross into shadow DOM are dropped, only reflect the valid
     // elements in IDL and in the content attribute.
@@ -661,8 +662,6 @@ <h2 id="lightDomHeading" aria-flowto="shadowChild1 shadowChild2">Light DOM Headi
       input.ariaActiveDescendantElement = first;
       first.parentElement.appendChild(first);
 
-      // This behaviour is currently under discussion by WHATWG.
-      // See: https://github.com/whatwg/html/pull/3917#issuecomment-527263562
       assert_equals(input.ariaActiveDescendantElement, first);
     }, "Reparenting.");
   </script>