Expand test defaults to include more states, include more blink test results

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

Index: content/browser/accessibility/dump_accessibility_tree_browsertest.cc
diff --git a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
index 76b617d753bab1d73bbf10133827416aab44efa9..f1418e0fa0478e4b4e00e0227a08c4f81bae22ed 100644
--- a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
+++ b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
@@ -56,10 +56,115 @@ typedef AccessibilityTreeFormatter::Filter Filter;
 class DumpAccessibilityTreeTest : public DumpAccessibilityTestBase {
  public:
   void AddDefaultFilters(std::vector<Filter>* filters) override {

[dmazzoni, 2017/06/13 20:25:33]

Since this function is getting long, declare it here and then
define it below the class. That lets you indent by 2 less,
might help wrapping a tiny bit but definitely will make the
class declaration a bit more readable.

[aleventhal, 2017/06/14 13:17:26]

Done.

-    filters->push_back(Filter(base::ASCIIToUTF16("FOCUSABLE"), Filter::ALLOW));
+    // TODO(aleventhal) Each platform deserves separate default filters.
+    // Windows
+    // Note: SELECTABLE, IA2_STATE_OPAQUE, LINKED off by default -- too noisy.
+    filters->push_back(Filter(base::ASCIIToUTF16("ALERT*"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("ANIMATED*"), Filter::ALLOW));

[dmazzoni, 2017/06/13 20:25:33]

Do we actually use this?

I'm tempted to get rid of ones we don't actually use *at all*,
or at least group them separately with a comment "not actually
used yet".

[aleventhal, 2017/06/14 13:17:25]

I want to keep them when we don't use them, to make sure we never do
unintentionally.

+    filters->push_back(Filter(base::ASCIIToUTF16("BUSY"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("CHECKED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("COLLAPSED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("DEFAULT"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("EXPANDED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("FLOATING"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("FOCUS*"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("HASPOPUP"), Filter::ALLOW));
+    // Too noisy:
+    // filters->push_back(Filter(base::ASCIIToUTF16("HOTTRACKED"),

[dmazzoni, 2017/06/13 20:25:33]

Maybe put all of the cpmmented-out off by default ones at the top or bottom.

Feel free to use more whitespace in this function, like:

//
// Windows
//

// Windows states to log by default
filters->push_back(
filters->push_back(
filters->push_back(

// Other Windows attributes

[aleventhal, 2017/06/14 13:17:26]

Done.

+    // Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("INVISIBLE"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("MARQUEED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("MIXED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("MOVEABLE"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("MULTISELECTABLE"), Filter::ALLOW));
+    // Produces false-positive for contentless elements, including <br>
+    // filters->push_back(Filter(base::ASCIIToUTF16("OFFSCREEN"),
+    // Filter::ALLOW));
     filters->push_back(Filter(base::ASCIIToUTF16("READONLY"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("PRESSED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("PROTECTED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("SELECTED"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("SIZEABLE"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("TRAVERSED"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("UNAVAILABLE"), Filter::ALLOW));
+    // IA2- No editable, opaque, single_line, selectable_text, vertical (noisy):
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_ACTIVE"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_ARMED"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_CHECKABLE"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_DEFUNCT"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_HORIZONTAL"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_ICONIFIED"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_INVALID_ENTRY"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_MODAL"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_MULTI_LINE"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_PINNED"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_REQUIRED"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_STALE"), Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("IA2_STATE_TRANSIENT"), Filter::ALLOW));
+
+    // Blink
+    filters->push_back(Filter(base::ASCIIToUTF16("check*"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("descript*"), Filter::ALLOW));

[dmazzoni, 2017/06/13 20:25:33]

I vote for either both nameFrom and descriptionFrom, or neither - but having
descriptionFrom but not nameFrom feels wrong

[aleventhal, 2017/06/14 13:17:25]

I wanted to keep descriptionFrom because it's rare, whereas nameFrom is crazy
common.

+    // TODO(aleventhal) Add disabled back after control mode refactor
+    // filters->push_back(Filter(base::ASCIIToUTF16("disabled"),
+    // Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("invalid"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("busy"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("collapsed"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("default"), Filter::ALLOW));
+    // TODO(aleventhal) Perhaps add editable and richlyEditable in later
+    // filters->push_back(Filter(base::ASCIIToUTF16("editable"),
+    // Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("expandable"), Filter::ALLOW));
+    // TODO(aleventhal) focus* causing a lot of changes, maybe add later
+    // filters->push_back(Filter(base::ASCIIToUTF16("focus*"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("haspopup"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("horizontal"), Filter::ALLOW));

[dmazzoni, 2017/06/13 20:25:33]

This should probably be an orientation enum {horizontal, veritical,
not_applicable}, right?

Does this one show up a lot? I think it might be distracting

[aleventhal, 2017/06/14 13:17:26]

Let's make it an enum at some point. For now, horizontal is rare so not very
distracting.

+    // Testing hovered would be flaky:
+    // filters->push_back(Filter(base::ASCIIToUTF16("hovered"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("invisible"), Filter::ALLOW));
+    // Linked is disallowed by default -- too noisy
+    // filters->push_back(Filter(base::ASCIIToUTF16("linked"), Filter::ALLOW));
+    // TODO(aleventhal) Add multiline in separate patch
+    // filters->push_back(Filter(base::ASCIIToUTF16("multiline"),
+    // Filter::ALLOW));
+    filters->push_back(
+        Filter(base::ASCIIToUTF16("multiselectable"), Filter::ALLOW));
+    // Produces false-positive for contentless elements, including <br>
+    // filters->push_back(Filter(base::ASCIIToUTF16("offscreen"),
+    // Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("protected"), Filter::ALLOW));
+    // TODO(aleventhal) Add readonly support back after control mode refactor
+    // filters->push_back(Filter(base::ASCIIToUTF16("read*"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("required"), Filter::ALLOW));
+    // TODO(aleventhal) Perhaps add editable and richlyEditable in later
+    // filters->push_back(Filter(base::ASCIIToUTF16("richly*"), Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("select*"), Filter::ALLOW));
+    // Vertical is a bit noisy -- we test for horizontal though:
+    // filters->push_back(Filter(base::ASCIIToUTF16("vertical"),
+    // Filter::ALLOW));
+    filters->push_back(Filter(base::ASCIIToUTF16("visited"), Filter::ALLOW));
+
+    // OS X
     filters->push_back(Filter(base::ASCIIToUTF16("roleDescription=*"),
                               Filter::ALLOW));
+
+    // General
     filters->push_back(Filter(base::ASCIIToUTF16("*=''"), Filter::DENY));
     // After denying empty values, because we want to allow name=''
     filters->push_back(