New issue
Advanced search Search tips

Issue 910056 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Focus does not traverse in proper order under chrome://settings/passwords on pressing down arrow key

Reported by vineetha...@etouch.net, Nov 29

Issue description

Chrome Version: 72.0.3625.0 (Official Build) Revision 2c59a8a07afb8b11354406e63270d1cbeb582c47-refs/branch-heads/3625@{#1}(32/64 bit)
OS: Windows (7,8,8.1,10), Mac(10.13.1, 10.13.6, 10.14.2), Linux(14.04 LTS) OS

Pre-condition: Multiple saved password entries should be present under chrome://settings/passwords

What steps will reproduce the problem?
(1) Launch Chrome, navigate to chrome://settings/passwords.
(2) Now delete any password entry present in the middle of the list(not the first one) and then undo the deletion.
(3) Press tab once and then Shift+tab to take focus onto the 3 dot(more action) icon of the first password entry.
(4) Press arrow key and observe that focus traverses through the password list along the 3-dot icon until the focus crosses the password entry that was restored.

Actual Result  : On pressing down arrow, focus shifts to the website name for the restored password entry.
Expected Result: Instead, on pressing arrow key, the focus should traverse along the 3 dot icon even after a password entry is restored.

This is a Regression issue seen from 'M-72' and providing the bisect info below:
Good Build: 72.0.3511.0 (Revision: 608212)
Bad Build:  72.0.3512.0 (Revision: 608630)

You are probably looking for a change made after 608561 (known good), but no later than 608562 (first known bad).
CHANGE-LOG 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/9b0436b7f7661e562fb0cb2d5d3aec70aa1d966f..292d851b752f8a97a347ecdf8e1f412afe9bcc37

Suspect: https://chromium.googlesource.com/chromium/src/+/292d851b752f8a97a347ecdf8e1f412afe9bcc37

@rbpotter: 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.

Thank You..!


 
ExpectedVideo.mp4
605 KB View Download
ActualVideo.mp4
618 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/689154ebc01a22bbf6e158a86fba792cd8333df2

commit 689154ebc01a22bbf6e158a86fba792cd8333df2
Author: rbpotter <rbpotter@chromium.org>
Date: Sat Dec 01 00:32:30 2018

Settings/History: bind |blurred| boolean to other items in list

In most cases, when a list is blurred and focus returns to the list,
focus returns to the last item that was blurred. However, in certain
cases like the linked bug, it is possible for focus to return to the
list but to a different item than the last one blurred. As a result,
the boolean tracking whether focus has left the list needs to be
shared by all items in the list. This CL changes it from a private
variable to a Polymer property data-bound to one parent property to
allow all items to access and update the same value.

Bug:  910056 
Change-Id: I763e0a11c87536aab6b189a2c71c18e6fa7d83e1
Reviewed-on: https://chromium-review.googlesource.com/c/1356233
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612874}
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/md_history/history_item.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/md_history/history_list.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/md_history/history_list.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/autofill_page/passwords_section.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/autofill_page/passwords_section.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/focus_row_behavior.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/search_engines_page/search_engines_list.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/search_engines_page/search_engines_list.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/search_engines_page/search_engines_page.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/search_engines_page/search_engines_page.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/site_settings/site_data.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/site_settings/site_data.js
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/site_settings/site_list.html
[modify] https://crrev.com/689154ebc01a22bbf6e158a86fba792cd8333df2/chrome/browser/resources/settings/site_settings/site_list.js

Labels: TE-Verified-M73 TE-Verified-73.0.3629.0
Update:

Rechecked the above issue on Windows (7,8,8.1,10), Mac(10.13.1, 10.13.6, 10.14.2), Linux(14.04 LTS) OS using latest Canary build #73.0.3629.0 and the issue is fixed. Kindly refer attached screen cast.

Thank you

FixedVideo.mp4
547 KB View Download
Status: Fixed (was: Assigned)
Labels: Merge-Request-72
Requesting a merge to M-72 since this is a P1 a11y regression bug and should be a safe merge (impacts a lot of files but all changes are very small).
Project Member

Comment 5 by sheriffbot@chromium.org, Dec 11

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 12

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5

commit ca7dfdc093a00bef71b32430fac212d0b3dcd7a5
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Dec 12 20:09:26 2018

Settings/History: bind |blurred| boolean to other items in list (M72)

In most cases, when a list is blurred and focus returns to the list,
focus returns to the last item that was blurred. However, in certain
cases like the linked bug, it is possible for focus to return to the
list but to a different item than the last one blurred. As a result,
the boolean tracking whether focus has left the list needs to be
shared by all items in the list. This CL changes it from a private
variable to a Polymer property data-bound to one parent property to
allow all items to access and update the same value.

Bug:  910056 
Change-Id: I763e0a11c87536aab6b189a2c71c18e6fa7d83e1
Reviewed-on: https://chromium-review.googlesource.com/c/1356233
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612874}(cherry picked from commit 689154ebc01a22bbf6e158a86fba792cd8333df2)
TBR: scottchen@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1372828
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#298}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/md_history/history_item.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/md_history/history_list.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/md_history/history_list.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/autofill_page/passwords_section.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/autofill_page/passwords_section.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/focus_row_behavior.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/search_engines_page/search_engines_list.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/search_engines_page/search_engines_list.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/search_engines_page/search_engines_page.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/search_engines_page/search_engines_page.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/site_settings/site_data.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/site_settings/site_data.js
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/site_settings/site_list.html
[modify] https://crrev.com/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5/chrome/browser/resources/settings/site_settings/site_list.js

Labels: TE-Verified-72.0.3626.17 TE-Verified-M72
Update:

Rechecked the above issue on Windows (7,8,8.1,10), Mac(10.13.1, 10.13.6, 10.14.2), Linux(14.04 LTS) OS using latest Beta build #72.0.3626.17 and the issue is fixed. Kindly refer attached screen cast.

Thank you
FixedVideo.mp4
401 KB View Download
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ca7dfdc093a00bef71b32430fac212d0b3dcd7a5

Commit: ca7dfdc093a00bef71b32430fac212d0b3dcd7a5
Author: rbpotter@chromium.org
Commiter: rbpotter@chromium.org
Date: 2018-12-12 20:09:26 +0000 UTC

Settings/History: bind |blurred| boolean to other items in list (M72)

In most cases, when a list is blurred and focus returns to the list,
focus returns to the last item that was blurred. However, in certain
cases like the linked bug, it is possible for focus to return to the
list but to a different item than the last one blurred. As a result,
the boolean tracking whether focus has left the list needs to be
shared by all items in the list. This CL changes it from a private
variable to a Polymer property data-bound to one parent property to
allow all items to access and update the same value.

Bug:  910056 
Change-Id: I763e0a11c87536aab6b189a2c71c18e6fa7d83e1
Reviewed-on: https://chromium-review.googlesource.com/c/1356233
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612874}(cherry picked from commit 689154ebc01a22bbf6e158a86fba792cd8333df2)
TBR: scottchen@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1372828
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#298}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment