Keyboard can be used to read App Info dialog for accessibility users.

See attached issue. Accessibility users can now tab through the
dialog, getting specific information (such as permissions), with the
screen reader reading each element.

A new subclass of Label has been created to ensure that non-
accessible users can get visual feedback if they use TAB to traverse
the dialog, since normal labels do not paint a text focus ring when
focused.

Future work:
 - Eliminate AppInfoPanel::CreateKeyValueField()
 - Merge AppInfoLabel functionality into views::Label

Bug: 996764
Change-Id: I97655c925c02b7d0b943f7fbb916ec1a87eed175
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1910871Reviewed-by: Caroline Rising <corising@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715054}

chrome/browser/ui/BUILD.gn

@@ -2622,6 +2622,8 @@ jumbo_static_library("ui") {
       "views/apps/app_info_dialog/app_info_footer_panel.h",
       "views/apps/app_info_dialog/app_info_header_panel.cc",
       "views/apps/app_info_dialog/app_info_header_panel.h",
+      "views/apps/app_info_dialog/app_info_label.cc",
+      "views/apps/app_info_dialog/app_info_label.h",
       "views/apps/app_info_dialog/app_info_panel.cc",
       "views/apps/app_info_dialog/app_info_panel.h",
       "views/apps/app_info_dialog/app_info_permissions_panel.cc",

chrome/browser/ui/views/apps/app_info_dialog/app_info_header_panel.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/grit/generated_resources.h"
@@ -81,9 +82,8 @@ void AppInfoHeaderPanel::CreateControls() {
   auto* vertical_info_container_ptr =
       AddChildView(std::move(vertical_info_container));
 
-  auto app_name_label = std::make_unique<views::Label>(
+  auto app_name_label = std::make_unique<AppInfoLabel>(
       base::UTF8ToUTF16(app_->name()), views::style::CONTEXT_DIALOG_TITLE);
-  app_name_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   auto* app_name_label_ptr =
       vertical_info_container_ptr->AddChildView(std::move(app_name_label));
 
@@ -92,6 +92,7 @@ void AppInfoHeaderPanel::CreateControls() {
         l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_WEB_STORE_LINK));
     view_in_store_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     view_in_store_link->set_listener(this);
+    view_in_store_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
     view_in_store_link_ = vertical_info_container_ptr->AddChildView(
         std::move(view_in_store_link));
   } else {

chrome/browser/ui/views/apps/app_info_dialog/app_info_label.cc

+// Copyright 2019 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/apps/app_info_dialog/app_info_label.h"
+
+#include "ui/gfx/canvas.h"
+
+AppInfoLabel::AppInfoLabel(const base::string16& text)
+    : AppInfoLabel(text,
+                   views::style::CONTEXT_LABEL,
+                   views::style::STYLE_PRIMARY) {}
+
+AppInfoLabel::AppInfoLabel(const base::string16& text,
+                           int text_context,
+                           int text_style,
+                           gfx::DirectionalityMode directionality_mode)
+    : Label(text, text_context, text_style, directionality_mode) {
+  // Note that ACCESSIBLE_ONLY only works in AccessiblePaneView, which these
+  // labels are not part of. So we need to mark them as ALWAYS because the user
+  // still needs to be able to tab-navigate them and get screen reader feedback.
+  SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+  SetHorizontalAlignment(gfx::ALIGN_LEFT);
+}
+
+void AppInfoLabel::PaintFocusRing(gfx::Canvas* canvas) const {
+  gfx::Rect focus_ring_bounds = GetTextBounds();
+  focus_ring_bounds.Intersect(GetLocalBounds());
+  canvas->DrawFocusRect(focus_ring_bounds);
+}
+
+void AppInfoLabel::OnFocus() {
+  Label::OnFocus();
+  SchedulePaint();
+}
+
+void AppInfoLabel::OnBlur() {
+  Label::OnBlur();
+  SchedulePaint();
+}

chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_
+#define CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_
+
+#include "ui/views/controls/label.h"
+
+// Label styled for use in AppInfo dialog so accessible users can step through
+// and have each line read.
+// TODO(dfried): merge functionality into views::Label.
+class AppInfoLabel : public views::Label {
+ public:
+  explicit AppInfoLabel(const base::string16& text);
+
+  // See documentation on views::Label::Label().
+  AppInfoLabel(const base::string16& text,
+               int text_context,
+               int text_style = views::style::STYLE_PRIMARY,
+               gfx::DirectionalityMode directionality_mode =
+                   gfx::DirectionalityMode::DIRECTIONALITY_FROM_TEXT);
+
+ private:
+  // views::Label:
+  void PaintFocusRing(gfx::Canvas* canvas) const override;
+  void OnFocus() override;
+  void OnBlur() override;
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_APPS_APP_INFO_DIALOG_APP_INFO_LABEL_H_

chrome/browser/ui/views/apps/app_info_dialog/app_info_panel.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/geometry/insets.h"
@@ -39,8 +40,7 @@ void AppInfoPanel::OpenLink(const GURL& url) {
 
 std::unique_ptr<views::Label> AppInfoPanel::CreateHeading(
     const base::string16& text) const {
-  auto label = std::make_unique<views::Label>(text);
-  label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  auto label = std::make_unique<AppInfoLabel>(text);
   label->SetFontList(ui::ResourceBundle::GetSharedInstance().GetFontList(
       ui::ResourceBundle::MediumFont));
   return label;

chrome/browser/ui/views/apps/app_info_dialog/app_info_panel.h

@@ -49,6 +49,8 @@ class AppInfoPanel : public views::View {
 
   // Given a key and a value, displays them side-by-side as a field and its
   // value.
+  // TODO(dfried): for ease of navigation, use GetStringFUTF16() and format the
+  // key and value together, eliminating this method.
   std::unique_ptr<views::View> CreateKeyValueField(
       std::unique_ptr<views::View> key,
       std::unique_ptr<views::View> value) const;

chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc

@@ -14,6 +14,7 @@
 #include "base/strings/string_split.h"
 #include "chrome/browser/apps/platform_apps/app_load_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/browser/api/device_permissions_manager.h"
@@ -142,15 +143,13 @@ class BulletedPermissionsList : public views::View {
     if (!revoke_callback.is_null())
       revoke_button = std::make_unique<RevokeButton>(revoke_callback, message);
 
-    auto permission_label = std::make_unique<views::Label>(message);
-    permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    auto permission_label = std::make_unique<AppInfoLabel>(message);
     permission_label->SetMultiLine(true);
     AddSinglePermissionBullet(false, std::move(permission_label),
                               std::move(revoke_button));
 
     for (const auto& submessage : submessages) {
-      auto sub_permission_label = std::make_unique<views::Label>(submessage);
-      sub_permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+      auto sub_permission_label = std::make_unique<AppInfoLabel>(submessage);
       sub_permission_label->SetElideBehavior(elide_behavior_for_submessages);
       AddSinglePermissionBullet(true, std::move(sub_permission_label), nullptr);
     }
@@ -212,11 +211,10 @@ void AppInfoPermissionsPanel::CreatePermissionsList() {
   if (!HasActivePermissionMessages() && GetRetainedDeviceCount() == 0 &&
       GetRetainedFileCount() == 0) {
     auto no_permissions_text =
-        std::make_unique<views::Label>(l10n_util::GetStringUTF16(
+        std::make_unique<AppInfoLabel>(l10n_util::GetStringUTF16(
             app_->is_extension()
                 ? IDS_APPLICATION_INFO_EXTENSION_NO_PERMISSIONS_TEXT
                 : IDS_APPLICATION_INFO_APP_NO_PERMISSIONS_TEXT));
-    no_permissions_text->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     AddChildView(std::move(no_permissions_text));
     return;
   }

chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_panel.cc

@@ -14,6 +14,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/views/apps/app_info_dialog/app_info_label.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/browser/extension_prefs.h"
@@ -132,9 +133,8 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl(
       text += base::ASCIIToUTF16(" ... ");
     }
 
-    auto description_label = std::make_unique<views::Label>(text);
+    auto description_label = std::make_unique<AppInfoLabel>(text);
     description_label->SetMultiLine(true);
-    description_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     description_and_labels_stack->AddChildView(std::move(description_label));
   }
 
@@ -143,6 +143,7 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl(
         l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_HOMEPAGE_LINK));
     homepage_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     homepage_link->set_listener(this);
+    homepage_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
     homepage_link_ =
         description_and_labels_stack->AddChildView(std::move(homepage_link));
   }
@@ -152,6 +153,7 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl(
         l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_LICENSES_BUTTON_TEXT));
     licenses_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     licenses_link->set_listener(this);
+    licenses_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
     licenses_link_ =
         description_and_labels_stack->AddChildView(std::move(licenses_link));
   }
@@ -169,13 +171,11 @@ void AppInfoSummaryPanel::AddDetailsControl(views::View* vertical_stack) {
           DISTANCE_RELATED_CONTROL_VERTICAL_SMALL));
 
   // Add the size.
-  auto size_title = std::make_unique<views::Label>(
+  auto size_title = std::make_unique<AppInfoLabel>(
       l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_SIZE_LABEL));
-  size_title->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
-  auto size_value = std::make_unique<views::Label>(
+  auto size_value = std::make_unique<AppInfoLabel>(
       l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_SIZE_LOADING_LABEL));
-  size_value->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   size_value_ = size_value.get();
   StartCalculatingAppSize();
 
@@ -184,13 +184,11 @@ void AppInfoSummaryPanel::AddDetailsControl(views::View* vertical_stack) {
 
   // The version doesn't make sense for bookmark apps.
   if (!app_->from_bookmark()) {
-    auto version_title = std::make_unique<views::Label>(
+    auto version_title = std::make_unique<AppInfoLabel>(
         l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_VERSION_LABEL));
-    version_title->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
-    auto version_value = std::make_unique<views::Label>(
+    auto version_value = std::make_unique<AppInfoLabel>(
         base::UTF8ToUTF16(app_->GetVersionForDisplay()));
-    version_value->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
     details_list->AddChildView(CreateKeyValueField(std::move(version_title),
                                                    std::move(version_value)));

chrome/browser/ui/views/apps/app_info_dialog/arc_app_info_links_panel.cc

@@ -34,6 +34,7 @@ ArcAppInfoLinksPanel::ArcAppInfoLinksPanel(Profile* profile,
       l10n_util::GetStringUTF16(IDS_ARC_APPLICATION_INFO_MANAGE_LINK));
   manage_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   manage_link->set_listener(this);
+  manage_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
   manage_link_ = AddChildView(std::move(manage_link));
 
   ArcAppListPrefs* const arc_prefs = ArcAppListPrefs::Get(profile_);