Settings: Fix cursor when hovering on People > Profile row

The row is sometimes clickable, depending on whether you are looking at
browser vs. OS settings, and whether ChromeOSAccountManager is enabled
or not.

When it is clickable, ensure the <div> elements are "actionable" so we
get the right cursor.

Also fix a problem where the avatar icon wasn't clickable in the
OS settings people section.

Screenshot from Chrome OS showing actionable row in OS settings:
https://screenshot.googleplex.com/ygC6Bn208dB

Screenshot from Linux (unchanged):
https://screenshot.googleplex.com/efW97d7diRY

Bug: 1002024
Test: added to browser_tests
Change-Id: Ib3a9f17e2d0f9b72eb6793e845c4679c141ccc1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1797122Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695406}

chrome/browser/resources/settings/chromeos/os_people_page/os_people_page.html

@@ -112,9 +112,12 @@
               <!-- Does not use <cr-link-row> due to custom aria label. -->
               <div id="profile-icon"
                   style="background-image: [[getIconImageSet_(
-                      profileIconUrl_)]]">
+                      profileIconUrl_)]]"
+                  on-click="onAccountManagerTap_"
+                  actionable$="[[isAccountManagerEnabled_]]">
               </div>
               <div class="middle two-line no-min-width"
+                  id="profile-row"
                   on-click="onAccountManagerTap_"
                   actionable$="[[isAccountManagerEnabled_]]">
                 <div class="flex text-elide settings-box-text">

chrome/browser/resources/settings/people_page/people_page.html

@@ -161,12 +161,14 @@
           <div id="picture-subpage-trigger" class="settings-box first two-line">
             <template is="dom-if" if="[[syncStatus]]">
               <div id="profile-icon" on-click="onProfileTap_"
-                  actionable$="[[allowChangePicture_]]"
+                  actionable$="[[isProfileActionable_]]"
                   style="background-image: [[getIconImageSet_(
                       profileIconUrl_)]]">
               </div>
               <div class="middle two-line no-min-width"
-                  on-click="onProfileTap_" actionable>
+                  id="profile-row"
+                  on-click="onProfileTap_"
+                  actionable$="[[isProfileActionable_]]">
                 <div class="flex text-elide settings-box-text">
                   <span id="profile-name">[[profileName_]]</span>
                   <div class="secondary" hidden="[[!syncStatus.signedIn]]">
@@ -181,7 +183,7 @@
 <if expr="chromeos">
                 <cr-icon-button class="subpage-arrow"
                     id="profile-subpage-arrow"
-                    hidden="[[!allowChangePicture_]]"
+                    hidden="[[!isProfileActionable_]]"
                     aria-label="$i18n{changePictureTitle}"
                     aria-describedby="profile-name"></cr-icon-button>
 </if>

chrome/browser/resources/settings/people_page/people_page.js

@@ -91,11 +91,11 @@ Polymer({
     profileIconUrl_: String,
 
     /**
-     * Whether clicking on the profile row should take the user to the
-     * "change picture" sub-page.
+     * Whether the profile row is clickable. The behavior depends on the
+     * platform.
      * @private
      */
-    allowChangePicture_: {
+    isProfileActionable_: {
       type: Boolean,
       value: function() {
         // On Chrome OS, only allow when SplitSettings is disabled.
@@ -379,8 +379,8 @@ Polymer({
   /** @private */
   onProfileTap_: function() {
     // <if expr="chromeos">
-    if (this.allowChangePicture_) {
-      // Testing allowChangePicture_ is simpler than conditionally removing
+    if (this.isProfileActionable_) {
+      // Testing isProfileActionable_ is simpler than conditionally removing
       // on-click handlers in the HTML.
       settings.navigateTo(settings.routes.CHANGE_PICTURE);
     }

chrome/test/data/webui/settings/chromeos/os_people_page_test.js

@@ -114,10 +114,14 @@ cr.define('settings_people_page', function() {
       await syncBrowserProxy.whenCalled('getSyncStatus');
       Polymer.dom.flush();
 
+      // Get page elements.
+      const profileIconEl = assert(peoplePage.$$('#profile-icon'));
+      const profileRowEl = assert(peoplePage.$$('#profile-row'));
+      const profileNameEl = assert(peoplePage.$$('#profile-name'));
+
       assertEquals(
-          browserProxy.fakeProfileInfo.name,
-          peoplePage.$$('#profile-name').textContent.trim());
-      const bg = peoplePage.$$('#profile-icon').style.backgroundImage;
+          browserProxy.fakeProfileInfo.name, profileNameEl.textContent.trim());
+      const bg = profileIconEl.style.backgroundImage;
       assertTrue(bg.includes(browserProxy.fakeProfileInfo.iconUrl));
 
       const iconDataUrl = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEA' +
@@ -126,11 +130,14 @@ cr.define('settings_people_page', function() {
           'profile-info-changed', {name: 'pushedName', iconUrl: iconDataUrl});
 
       Polymer.dom.flush();
-      assertEquals(
-          'pushedName', peoplePage.$$('#profile-name').textContent.trim());
-      const newBg = peoplePage.$$('#profile-icon').style.backgroundImage;
+      assertEquals('pushedName', profileNameEl.textContent.trim());
+      const newBg = profileIconEl.style.backgroundImage;
       assertTrue(newBg.includes(iconDataUrl));
 
+      // Profile row items aren't actionable.
+      assertFalse(profileIconEl.hasAttribute('actionable'));
+      assertFalse(profileRowEl.hasAttribute('actionable'));
+
       // Sub-page trigger is hidden.
       assertTrue(peoplePage.$$('#account-manager-subpage-trigger').hidden);
     });
@@ -147,11 +154,15 @@ cr.define('settings_people_page', function() {
       await syncBrowserProxy.whenCalled('getSyncStatus');
       Polymer.dom.flush();
 
+      // Get page elements.
+      const profileIconEl = assert(peoplePage.$$('#profile-icon'));
+      const profileRowEl = assert(peoplePage.$$('#profile-row'));
+      const profileNameEl = assert(peoplePage.$$('#profile-name'));
+
       chai.assert.include(
-          peoplePage.$$('#profile-icon').style.backgroundImage,
+          profileIconEl.style.backgroundImage,
           'data:image/png;base64,primaryAccountPicData');
-      assertEquals(
-          'Primary Account', peoplePage.$$('#profile-name').textContent.trim());
+      assertEquals('Primary Account', profileNameEl.textContent.trim());
 
       // Rather than trying to mock cr.sendWithPromise('getPluralString', ...)
       // just force an update.
@@ -160,6 +171,10 @@ cr.define('settings_people_page', function() {
           'primary@gmail.com, +2 more accounts',
           peoplePage.$$('#profile-label').textContent.trim());
 
+      // Profile row items are actionable.
+      assertTrue(profileIconEl.hasAttribute('actionable'));
+      assertTrue(profileRowEl.hasAttribute('actionable'));
+
       // Sub-page trigger is shown.
       const subpageTrigger = peoplePage.$$('#account-manager-subpage-trigger');
       assertFalse(subpageTrigger.hidden);

chrome/test/data/webui/settings/people_page_test.js

@@ -699,6 +699,8 @@ cr.define('settings_people_page', function() {
         // Profile row items aren't actionable.
         const profileIcon = assert(peoplePage.$$('#profile-icon'));
         assertFalse(profileIcon.hasAttribute('actionable'));
+        const profileRow = assert(peoplePage.$$('#profile-row'));
+        assertFalse(profileRow.hasAttribute('actionable'));
         const subpageArrow = assert(peoplePage.$$('#profile-subpage-arrow'));
         assertFalse(subpageArrow.hasAttribute('actionable'));