Accessibility for Autofill Popup View in native code

chrome/browser/ui/views/autofill/autofill_popup_view_views.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/autofill/autofill_popup_view_views.h"
 
 #include "chrome/browser/ui/autofill/autofill_popup_controller.h"
 #include "chrome/browser/ui/autofill/autofill_popup_layout_model.h"
 #include "components/autofill/core/browser/popup_item_ids.h"
 #include "components/autofill/core/browser/suggestion.h"
+#include "ui/accessibility/ax_node_data.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/text_utils.h"
 #include "ui/views/border.h"
 #include "ui/views/widget/widget.h"
 
 namespace autofill {
 
 AutofillPopupViewViews::AutofillPopupViewViews(
     AutofillPopupController* controller,
     views::Widget* parent_widget)
     : AutofillPopupBaseView(controller, parent_widget),
-      controller_(controller) {}
+      controller_(controller) {
+  for (size_t i = 0; i < controller_->GetLineCount(); ++i) {

[Mathieu, 2017/02/02 22:07:44]

if it's a separator, let's not create a child view for it but rather pass the
information to the next child, which will now have a top sided border in their
constructor:

https://cs.chromium.org/chromium/src/ui/views/border.h?l=88&gs=cpp%253Aviews%...

[csashi, 2017/02/02 23:27:18]

Do we want an accessibility handler for the separator? If yes, would we not need
a separate child view to return different accessibility data for the separator.
Right now I am using ui::AX_ROLE_SPLITTER but not sure if that is the right
accessibility data.

+    AutofillPopupChildView *child = new AutofillPopupChildView(controller, i);
+    AddChildViewAt(child, static_cast<int>(i));
+  }
+}
 
 AutofillPopupViewViews::~AutofillPopupViewViews() {}
 
 void AutofillPopupViewViews::Show() {
   DoShow();
 }
 
 void AutofillPopupViewViews::Hide() {
   // The controller is no longer valid after it hides us.
   controller_ = NULL;

[Mathieu, 2017/02/02 22:07:44]

Since DoHide just kills us, I wonder if this is needed. Anyway, maybe not in the
scope of this

[csashi, 2017/02/02 23:27:18]

Done.

-
+  for (int i = 0; i < child_count(); ++i) {
+    (static_cast<AutofillPopupChildView*>(child_at(i)))->clear_controller();
+  }
   DoHide();
 }
 
 void AutofillPopupViewViews::UpdateBoundsAndRedrawPopup() {
   DoUpdateBoundsAndRedrawPopup();
 }
 
 void AutofillPopupViewViews::OnPaint(gfx::Canvas* canvas) {
   if (!controller_)
     return;
 
   canvas->DrawColor(GetNativeTheme()->GetSystemColor(

[Mathieu, 2017/02/02 19:30:15]

I think we could use set_background in the constructor of this View (or a
utility function to add one)

[Mathieu, 2017/02/02 22:07:45]

In the constructor: 

set_background(views::Background::CreateSolidBackground(GetNativeTheme()->
       
GetSystemColor(ui::NativeTheme::kColorId_ResultsTableNormalBackground)));

(I don't have it quite working yet but it's the idea).

[csashi, 2017/02/02 23:27:18]

Done.

[csashi, 2017/02/02 23:27:18]

Done.

       ui::NativeTheme::kColorId_ResultsTableNormalBackground));
   OnPaintBorder(canvas);

[Mathieu, 2017/02/02 19:30:15]

I think we could SetBorder in the constructor

[Mathieu, 2017/02/02 22:07:45]

In the constructor (color to be confirmed):

SetBorder(views::CreateSolidSidedBorder(1, 1, 1, 1, GetNativeTheme()->
       
GetSystemColor(ui::NativeTheme::kColorId_ResultsTableDimmedBackground)));

-
-  for (size_t i = 0; i < controller_->GetLineCount(); ++i) {
-    gfx::Rect line_rect = controller_->layout_model().GetRowBounds(i);
-
-    if (controller_->GetSuggestionAt(i).frontend_id ==
-        POPUP_ITEM_ID_SEPARATOR) {
-      canvas->FillRect(
-          line_rect,
-          GetNativeTheme()->GetSystemColor(
-              ui::NativeTheme::kColorId_ResultsTableNormalDimmedText));
-    } else {
-      DrawAutofillEntry(canvas, i, line_rect);
-    }
+  for (int i = 0; i < child_count(); ++i) {
+    (static_cast<AutofillPopupChildView*>(child_at(i)))->OnPaint(canvas);

[Mathieu, 2017/02/02 19:30:15]

Since this is the last thing, we shouldn't need to override OnPaint at all in
this class. The framework will call OnPaint on the popup and all its children

[csashi, 2017/02/02 23:27:18]

I thought so too, but that didn't work. Not sure what you mean by "framework",
but View::OnPaint does not call paint on children.

   }
 }
 
 void AutofillPopupViewViews::InvalidateRow(size_t row) {
-  SchedulePaintInRect(controller_->layout_model().GetRowBounds(row));
+  child_at(row)->SchedulePaint();
+}
+
+void AutofillPopupViewViews::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  LOG(VERBOSE) << "AutofillPopupViewViews::GetAccessibleNodeData";
+}
+
+AutofillPopupChildView::~AutofillPopupChildView() {}
+
+void AutofillPopupChildView::OnPaint(gfx::Canvas* canvas) {
+  if (!controller_)
+    return;
+  const gfx::Rect line_rect = controller_->layout_model().GetRowBounds(index_);
+  if (controller_->GetSuggestionAt(index_).frontend_id ==
+      POPUP_ITEM_ID_SEPARATOR) {

[Mathieu, 2017/02/02 22:07:45]

As mentioned above, let's not draw separators but rely on SidedBorder's

[csashi, 2017/02/02 23:27:18]

Same comment as above. Doesn't this depend on whether we want accessibility
handler for the separator?

+    canvas->FillRect(
+        line_rect,
+        GetNativeTheme()->GetSystemColor(
+            ui::NativeTheme::kColorId_ResultsTableNormalDimmedText));
+  } else {
+    DrawAutofillEntry(canvas, index_, line_rect);
+  }
 }
 
 /**
 * Autofill entries in ltr.
 *
 * ............................................................................
 * . ICON | HTTP WARNING MESSAGE VALUE                                | LABEL .
 * ............................................................................
 * . OTHER AUTOFILL ENTRY VALUE |                                LABEL | ICON .
 * ............................................................................
 *
 * Autofill entries in rtl.
 *
 * ............................................................................
 * . LABEL |                                HTTP WARNING MESSAGE VALUE | ICON .
 * ............................................................................
 * . ICON | LABEL                                | OTHER AUTOFILL ENTRY VALUE .
 * ............................................................................
 *
 * Anyone who wants to modify the code below, remember to make sure that HTTP
 * warning entry displays right. To trigger the warning message entry, enable
 * #mark-non-secure-as flag as "display form warning", go to goo.gl/CEIjc6 with
 * stored autofill info and check for credit card or password forms.
 */
-void AutofillPopupViewViews::DrawAutofillEntry(gfx::Canvas* canvas,
+void AutofillPopupChildView::DrawAutofillEntry(gfx::Canvas* canvas,
                                                int index,
                                                const gfx::Rect& entry_rect) {
   canvas->FillRect(
       entry_rect,
       GetNativeTheme()->GetSystemColor(
           controller_->GetBackgroundColorIDForRow(index)));
 
   const bool is_http_warning =
       (controller_->GetSuggestionAt(index).frontend_id ==
        POPUP_ITEM_ID_HTTP_NOT_SECURE_WARNING_MESSAGE);
@@ skipping 76 matching lines @@
         controller_->GetElidedLabelAt(index),
         controller_->layout_model().GetLabelFontListForRow(index),
         GetNativeTheme()->GetSystemColor(
             ui::NativeTheme::kColorId_ResultsTableNormalDimmedText),
         gfx::Rect(label_x_align_left, entry_rect.y(), label_width,
                   entry_rect.height()),
         text_align);
   }
 }
 
+void AutofillPopupChildView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  LOG(VERBOSE) << "AutofillPopupChildView::GetAccessibleNodeData";
+  if (!controller_)
+    return;
+  if (controller_->GetSuggestionAt(index_).frontend_id ==
+      POPUP_ITEM_ID_SEPARATOR) {
+    node_data->role = ui::AX_ROLE_SPLITTER;
+    node_data->AddStateFlag(ui::AX_STATE_READ_ONLY);
+    return;
+  }
+  node_data->role = ui::AX_ROLE_TEXT_FIELD;
+  node_data->SetName(controller_->GetElidedLabelAt(index_));
+  node_data->SetValue(controller_->GetElidedValueAt(index_));
+  node_data->AddStateFlag(ui::AX_STATE_EDITABLE);
+}
+
 AutofillPopupView* AutofillPopupView::Create(
     AutofillPopupController* controller) {
   views::Widget* observing_widget =
       views::Widget::GetTopLevelWidgetForNativeView(
           controller->container_view());
 
   // If the top level widget can't be found, cancel the popup since we can't
   // fully set it up.
   if (!observing_widget)
     return NULL;
 
   return new AutofillPopupViewViews(controller, observing_widget);
 }
 
 }  // namespace autofill