New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment
link

Issue 908325: Regression : Click effect is not seen on 'Customize fonts' arrow icon in chrome://settings page.

Reported by rp...@etouch.net, Nov 26

Issue description

Chrome version: 72.0.3621.0 (Official Build)Revision a10b0af074b5c7c088e27fb9d801d568a85b56a1-refs/branch-heads/3621@{#1}(32/64-bit)
OS: Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.14.2,10.13.1,10.13.6)

What steps will reproduce the problem?
1. Launch chrome,navigate to chrome://settings/
2. Now press 'Tab' key to bring focus on 'Customize fonts' arrow icon 
3. Now press 'Space-bar' key from keyboard and observe the focus on arrow icon
 
Actual: Click effect is not seen on 'Customize fonts' arrow icon after pressing 'Space-bar' key from keyboard
Expected: Click effect should be seen on 'Customize fonts' arrow icon after pressing 'Space-bar' key from keyboard

This is regression issue, broken in ‘M 63’ and below is the bisect info :
Good build: 63.0.3221.0  (Revision: 503309).
Bad build: 63.0.3222.0 (Revision: 503582).

You are probably looking for a change made after 503557 (known good), but no later than 503558 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/43cda23b74991c6b1fc061e11ad7a0d29532c0da..39a1080f56c175f7a6792c205e9cefd5c7c5a804

Suspect : https://chromium.googlesource.com/chromium/src/+/39a1080f56c175f7a6792c205e9cefd5c7c5a804

From the CL above, assigning the issue to the concern owner 

@scottchen- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.
 
Actual_video.mov
2.6 MB View Download
Expected_video.mov
2.5 MB View Download

Comment 1 by scottchen@chromium.org, Dec 7

Cc: johntlee@chromium.org dpa...@chromium.org
Labels: -Pri-1 Pri-2
@dpapad it seems like there's inconsistency between different subpage triggers, since some are using <div> + <paper-icon-button-light>, which produces a "double-ripple" animation when clicked, where as some subpage triggers are <cr-link-row>s which implement PaperRippleBehavior and the icon's just a <div>.

I think according to https://material-components.github.io/material-components-web-catalog/#/component/icon-button, a focused icon button is supposed to show 2 ripples. 

Should we just swap the <div> icon in cr-link-row with a <paper-icon-button-light> instead of trying to re-implement with PaperRippleBehavior?

Comment 2 by dpa...@chromium.org, Dec 8

Are you referring to the difference between "Customise fonts" (which shows a double ripple, unlike what the original report says) and "Manage search engines" which does not? See screencast

I think it makes sense to align those rows, to use the same element (cr-link-row?) since they look identical anyway. Other than that, I don't really mind whether a 2nd ripple is showing or not, but being consistent seems more important.
ripple.mp4
714 KB View Download

Comment 3 by scottchen@chromium.org, Dec 13

Owner: johntlee@google.com
Yeah that's the difference I'm referring to.

Regarding the second ripple, Google Material's demos seem to have it, we could check with namratakannan@ if we wish to align with that.

but either way, I think making them both cr-link-row would be a good first step, so whichever way we want to go with the second-ripple we have only 1 thing to update.

Comment 4 by johntlee@chromium.org, Dec 13

Owner: johntlee@chromium.org

Comment 5 by bugdroid1@chromium.org, Dec 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b167c24cd4ac3678e3563c915688b48df9fbb00

commit 6b167c24cd4ac3678e3563c915688b48df9fbb00
Author: John Lee <johntlee@chromium.org>
Date: Sat Dec 15 02:52:46 2018

WebUI: Migrate 'Manage search engines' button to cr-link-row

Bug: 908325
Change-Id: Ifd84b8fcef8ed1ad074909cfecce6a7cfef31d52
Reviewed-on: https://chromium-review.googlesource.com/c/1376666
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616929}
[modify] https://crrev.com/6b167c24cd4ac3678e3563c915688b48df9fbb00/chrome/browser/resources/settings/search_page/search_page.html
[modify] https://crrev.com/6b167c24cd4ac3678e3563c915688b48df9fbb00/chrome/browser/resources/settings/search_page/search_page.js

Comment 6 by rp...@virtusa.com, Dec 17

Labels: TE-Verified-M73 TE-Verified-73.0.3643.0
Update :
Rechecked the above issue on Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.13.1,10.13.6,10.14.1) using latest Canary build : 73.0.3643.0 and the issue is Fixed.Hence adding TE Verified Labels.

Kindly refer the attached screen cast.

Thank you..!!
Fixed_video.mov
4.3 MB View Download

Sign in to add a comment