[Extensions A11y] Update a11y text for inline installation dialog

The extension inline install dialog contains a section for the webstore
ratings. This is rendered as a star rating (n / 5 filled stars) and a
user count displayed in parentheses. For example,
***** (345)

Unfortunately, the accessible text for this is lacking - each star
(filled or not) simply says "graphic", followed by the number. So the
content read by a screen reader would be:
"graphic graphic graphic graphic graphic 345"
which is very unhelpful.

Introduce a custom view for the ratings section, which hides the star
graphics and the user label from the accessibility tree. Instead,
provide custom accessible text to give meaning. Now, the screen reader
will read:
"Rated 5.0 by 345 users"

Bug: 747624

Change-Id: I96d829efd098cc6795389b3190f0c2a901f55a93
Reviewed-on: https://chromium-review.googlesource.com/782711
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520693}

chrome/app/generated_resources.grd

@@ -3507,6 +3507,12 @@ From <ph name="DOWNLOAD_DOMAIN">$3<ex>example.com</ex></ph>
       <message name="IDS_EXTENSION_RATING_COUNT" desc="Number of ratings an app or extensions has (displayed after star icons)">
         (<ph name="RATING_COUNT">$1<ex>46</ex></ph>)
       </message>
+      <message name="IDS_EXTENSION_PROMPT_RATING_ACCESSIBLE_TEXT" desc="The text read by a screen reader for the ratings section of an extension prompt, indicating to the user what the average rating for an extension is and how many times it has been rated. [ICU Syntax]">
+        Rated <ph name="AVERAGE_RATING">{0, number,0.0}<ex>3.2</ex></ph> by <ph name="NUMBER_OF_RATINGS">{1, plural, =1 {one user} other {# users}}<ex>400 users</ex></ph>.
+      </message>
+      <message name="IDS_EXTENSION_PROMPT_NO_RATINGS_ACCESSIBLE_TEXT" desc="The text read by a screen reader for the ratings section of an extension prompt when there haven't been any user ratings for the extension.">
+        Not yet rated by any users.
+      </message>
       <message name="IDS_EXTENSION_USER_COUNT" desc="Number of users an app or extension has">
         <ph name="USER_COUNT">$1<ex>10,479</ex></ph> users
       </message>

chrome/browser/extensions/extension_install_prompt.h

@@ -173,6 +173,9 @@ class ExtensionInstallPrompt {
     const gfx::Image& icon() const { return icon_; }
     void set_icon(const gfx::Image& icon) { icon_ = icon; }
 
+    double average_rating() const { return average_rating_; }
+    int rating_count() const { return rating_count_; }
+
     bool has_webstore_data() const { return has_webstore_data_; }
 
    private:

chrome/browser/ui/views/extensions/extension_install_dialog_view.cc

@@ -13,6 +13,7 @@
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/i18n/message_formatter.h"
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
@@ -85,6 +86,73 @@ constexpr int kExternalInstallLeftColumnWidth = 350;
 // Time delay before the install button is enabled after initial display.
 int g_install_delay_in_ms = 500;
 
+// A custom view to contain the ratings information (stars, ratings count, etc).
+// With screen readers, this will handle conveying the information properly
+// (i.e., "Rated 4.2 stars by 379 reviews" rather than "image image...379").
+class RatingsView : public views::View {
+ public:
+  RatingsView(double rating, int rating_count)
+      : rating_(rating), rating_count_(rating_count) {
+    set_id(ExtensionInstallDialogView::kRatingsViewId);
+    SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal));
+  }
+  ~RatingsView() override {}
+
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
+    node_data->role = ui::AX_ROLE_STATIC_TEXT;
+    base::string16 accessible_text;
+    if (rating_count_ == 0) {
+      accessible_text = l10n_util::GetStringUTF16(
+          IDS_EXTENSION_PROMPT_NO_RATINGS_ACCESSIBLE_TEXT);
+    } else {
+      accessible_text = base::i18n::MessageFormatter::FormatWithNumberedArgs(
+          l10n_util::GetStringUTF16(
+              IDS_EXTENSION_PROMPT_RATING_ACCESSIBLE_TEXT),
+          rating_, rating_count_);
+    }
+    node_data->SetName(accessible_text);
+  }
+
+ private:
+  double rating_;
+  int rating_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(RatingsView);
+};
+
+// A custom view for the ratings star image that will be ignored by screen
+// readers (since the RatingsView handles the context).
+class RatingStar : public views::ImageView {
+ public:
+  explicit RatingStar(const gfx::ImageSkia& image) { SetImage(image); }
+  ~RatingStar() override {}
+
+  // views::ImageView:
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
+    node_data->role = ui::AX_ROLE_IGNORED;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RatingStar);
+};
+
+// A custom view for the ratings label that will be ignored by screen readers
+// (since the RatingsView handles the context).
+class RatingLabel : public views::Label {
+ public:
+  RatingLabel(const base::string16& text, int text_context)
+      : views::Label(text, text_context, views::style::STYLE_PRIMARY) {}
+  ~RatingLabel() override {}
+
+  // views::Label:
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
+    node_data->role = ui::AX_ROLE_IGNORED;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RatingLabel);
+};
+
 // Get the appropriate indentation for an item if its parent is using bullet
 // points. If the parent is using bullets for its items, then a padding of one
 // unit will make the child item (which has no bullet) look like a sibling of
@@ -98,9 +166,7 @@ int GetLeftPaddingForBulletedItems(bool parent_bulleted) {
 
 void AddResourceIcon(const gfx::ImageSkia* skia_image, void* data) {
   views::View* parent = static_cast<views::View*>(data);
-  views::ImageView* image_view = new views::ImageView();
-  image_view->SetImage(*skia_image);
-  parent->AddChildView(image_view);
+  parent->AddChildView(new RatingStar(*skia_image));
 }
 
 // Creates a string for displaying |message| to the user. If it has to look
@@ -238,9 +304,8 @@ void ExtensionInstallDialogView::InitView() {
 
   if (prompt_->has_webstore_data()) {
     layout->StartRow(0, column_set_id);
-    views::View* rating = new views::View();
-    rating->SetLayoutManager(
-        new views::BoxLayout(views::BoxLayout::kHorizontal));
+    RatingsView* rating =
+        new RatingsView(prompt_->average_rating(), prompt_->rating_count());
     layout->AddView(rating);
     prompt_->AppendRatingStars(AddResourceIcon, rating);
 
@@ -252,8 +317,7 @@ void ExtensionInstallDialogView::InitView() {
       rating_text_context = user_count_text_context = CONTEXT_DEPRECATED_SMALL;
     }
     views::Label* rating_count =
-        new views::Label(prompt_->GetRatingCount(), rating_text_context,
-                         views::style::STYLE_PRIMARY);
+        new RatingLabel(prompt_->GetRatingCount(), rating_text_context);
     // Add some space between the stars and the rating count.
     rating_count->SetBorder(views::CreateEmptyBorder(0, 2, 0, 0));
     rating->AddChildView(rating_count);

chrome/browser/ui/views/extensions/extension_install_dialog_view.h

@@ -39,6 +39,9 @@ class Link;
 class ExtensionInstallDialogView : public views::DialogDelegateView,
                                    public views::LinkListener {
  public:
+  // The views::View::id of the ratings section in the dialog.
+  static const int kRatingsViewId = 1;
+
   ExtensionInstallDialogView(
       Profile* profile,
       content::PageNavigator* navigator,

chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
@@ -24,12 +25,14 @@
 #include "chrome/browser/ui/webui/extensions/extension_settings_handler.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension_test_util.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/constrained_window/constrained_window_views.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/permissions/permission_message_provider.h"
 #include "extensions/common/permissions/permissions_data.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
@@ -41,28 +44,25 @@ using extensions::PermissionMessages;
 
 class ExtensionInstallDialogViewTestBase : public ExtensionBrowserTest {
  protected:
-  explicit ExtensionInstallDialogViewTestBase(
-      ExtensionInstallPrompt::PromptType prompt_type);
-  ~ExtensionInstallDialogViewTestBase() override {}
+  ExtensionInstallDialogViewTestBase();
 
   void SetUpOnMainThread() override;
 
-  // Creates and returns an install prompt of |prompt_type_|.
-  std::unique_ptr<ExtensionInstallPrompt::Prompt> CreatePrompt();
+  // Creates and returns an install prompt of |prompt_type|.
+  std::unique_ptr<ExtensionInstallPrompt::Prompt> CreatePrompt(
+      ExtensionInstallPrompt::PromptType prompt_type);
 
   content::WebContents* web_contents() { return web_contents_; }
 
  private:
   const extensions::Extension* extension_;
-  ExtensionInstallPrompt::PromptType prompt_type_;
   content::WebContents* web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogViewTestBase);
 };
 
-ExtensionInstallDialogViewTestBase::ExtensionInstallDialogViewTestBase(
-    ExtensionInstallPrompt::PromptType prompt_type)
-    : extension_(nullptr), prompt_type_(prompt_type), web_contents_(nullptr) {}
+ExtensionInstallDialogViewTestBase::ExtensionInstallDialogViewTestBase()
+    : extension_(nullptr), web_contents_(nullptr) {}
 
 void ExtensionInstallDialogViewTestBase::SetUpOnMainThread() {
   ExtensionBrowserTest::SetUpOnMainThread();
@@ -74,9 +74,10 @@ void ExtensionInstallDialogViewTestBase::SetUpOnMainThread() {
 }
 
 std::unique_ptr<ExtensionInstallPrompt::Prompt>
-ExtensionInstallDialogViewTestBase::CreatePrompt() {
+ExtensionInstallDialogViewTestBase::CreatePrompt(
+    ExtensionInstallPrompt::PromptType prompt_type) {
   std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt(
-      new ExtensionInstallPrompt::Prompt(prompt_type_));
+      new ExtensionInstallPrompt::Prompt(prompt_type));
   prompt->set_extension(extension_);
 
   std::unique_ptr<ExtensionIconManager> icon_manager(
@@ -88,8 +89,7 @@ ExtensionInstallDialogViewTestBase::CreatePrompt() {
 
 class ScrollbarTest : public ExtensionInstallDialogViewTestBase {
  protected:
-  ScrollbarTest();
-  ~ScrollbarTest() override {}
+  ScrollbarTest() {}
 
   bool IsScrollbarVisible(
       std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt);
@@ -98,11 +98,6 @@ class ScrollbarTest : public ExtensionInstallDialogViewTestBase {
   DISALLOW_COPY_AND_ASSIGN(ScrollbarTest);
 };
 
-ScrollbarTest::ScrollbarTest()
-    : ExtensionInstallDialogViewTestBase(
-          ExtensionInstallPrompt::PERMISSIONS_PROMPT) {
-}
-
 bool ScrollbarTest::IsScrollbarVisible(
     std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt) {
   ExtensionInstallDialogView* dialog = new ExtensionInstallDialogView(
@@ -128,7 +123,8 @@ IN_PROC_BROWSER_TEST_F(ScrollbarTest, LongPromptScrollbar) {
     permissions.push_back(PermissionMessage(permission_string,
                                             PermissionIDSet()));
   }
-  std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt = CreatePrompt();
+  std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt =
+      CreatePrompt(ExtensionInstallPrompt::PERMISSIONS_PROMPT);
   prompt->AddPermissions(permissions,
                          ExtensionInstallPrompt::REGULAR_PERMISSIONS);
   ASSERT_TRUE(IsScrollbarVisible(std::move(prompt)))
@@ -143,7 +139,8 @@ IN_PROC_BROWSER_TEST_F(ScrollbarTest, ScrollbarRegression) {
   PermissionMessages permissions;
   permissions.push_back(PermissionMessage(permission_string,
                                           PermissionIDSet()));
-  std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt = CreatePrompt();
+  std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt =
+      CreatePrompt(ExtensionInstallPrompt::PERMISSIONS_PROMPT);
   prompt->AddPermissions(permissions,
                          ExtensionInstallPrompt::REGULAR_PERMISSIONS);
   ASSERT_FALSE(IsScrollbarVisible(std::move(prompt))) << "Scrollbar is visible";
@@ -152,16 +149,14 @@ IN_PROC_BROWSER_TEST_F(ScrollbarTest, ScrollbarRegression) {
 class ExtensionInstallDialogViewTest
     : public ExtensionInstallDialogViewTestBase {
  protected:
-  ExtensionInstallDialogViewTest()
-      : ExtensionInstallDialogViewTestBase(
-            ExtensionInstallPrompt::INSTALL_PROMPT) {}
-  ~ExtensionInstallDialogViewTest() override {}
+  ExtensionInstallDialogViewTest() {}
 
   views::DialogDelegateView* CreateAndShowPrompt(
       ExtensionInstallPromptTestHelper* helper) {
     std::unique_ptr<ExtensionInstallDialogView> dialog(
-        new ExtensionInstallDialogView(profile(), web_contents(),
-                                       helper->GetCallback(), CreatePrompt()));
+        new ExtensionInstallDialogView(
+            profile(), web_contents(), helper->GetCallback(),
+            CreatePrompt(ExtensionInstallPrompt::INSTALL_PROMPT)));
     views::DialogDelegateView* delegate_view = dialog.get();
 
     views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget(
@@ -343,3 +338,65 @@ IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewInteractiveBrowserTest,
                             base::ASCIIToUTF16("Detailed permission 3")});
   RunDialog();
 }
+
+class ExtensionInstallDialogRatingsSectionTest
+    : public ExtensionInstallDialogViewTest {
+ public:
+  ExtensionInstallDialogRatingsSectionTest() {}
+
+  void TestRatingsSectionA11y(int num_ratings,
+                              double average_rating,
+                              const std::string& expected_text);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogRatingsSectionTest);
+};
+
+void ExtensionInstallDialogRatingsSectionTest::TestRatingsSectionA11y(
+    int num_ratings,
+    double average_rating,
+    const std::string& expected_text) {
+  SCOPED_TRACE(base::StringPrintf(
+      "Testing with %d ratings, %f average rating. Expected text: '%s'.",
+      num_ratings, average_rating, expected_text.c_str()));
+  std::unique_ptr<ExtensionInstallPrompt::Prompt> prompt =
+      CreatePrompt(ExtensionInstallPrompt::INLINE_INSTALL_PROMPT);
+  prompt->SetWebstoreData("1,234", true, average_rating, num_ratings);
+
+  ExtensionInstallDialogView* dialog = new ExtensionInstallDialogView(
+      profile(), web_contents(),
+      base::Bind([](ExtensionInstallPrompt::Result result) {}),
+      std::move(prompt));
+
+  views::Widget* modal_dialog = views::DialogDelegate::CreateDialogWidget(
+      dialog, nullptr,
+      platform_util::GetViewForWindow(browser()->window()->GetNativeWindow()));
+  modal_dialog->Show();
+
+  views::View* rating_view =
+      dialog->GetViewByID(ExtensionInstallDialogView::kRatingsViewId);
+  ASSERT_TRUE(rating_view);
+  {
+    ui::AXNodeData node_data;
+    rating_view->GetAccessibleNodeData(&node_data);
+    EXPECT_EQ(ui::AX_ROLE_STATIC_TEXT, node_data.role);
+    EXPECT_EQ(expected_text, node_data.GetStringAttribute(ui::AX_ATTR_NAME));
+  }
+
+  for (int i = 0; i < rating_view->child_count(); ++i) {
+    views::View* child = rating_view->child_at(i);
+    ui::AXNodeData node_data;
+    child->GetAccessibleNodeData(&node_data);
+    EXPECT_EQ(ui::AX_ROLE_IGNORED, node_data.role);
+  }
+
+  modal_dialog->Close();
+  base::RunLoop().RunUntilIdle();
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogRatingsSectionTest,
+                       RatingsSectionA11y) {
+  TestRatingsSectionA11y(400, 3.297, "Rated 3.3 by 400 users.");
+  TestRatingsSectionA11y(1, 1.0, "Rated 1.0 by one user.");
+  TestRatingsSectionA11y(0, 0.0, "Not yet rated by any users.");
+}