New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 699001 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression:Two Focus are seen in the manage search engines page of chrome://md-settings.

Project Member Reported by sahitya....@techmahindra.com, Mar 7 2017

Issue description

Chrome Version: 58.0.3029.6
OS: Ubuntu 14.04,Windows

What steps will reproduce the problem?
(1)Launch chrome and login with valid credentials.
(2)Navigate to the chrome://md-settings>>manage search engines page(their should be more than 30+ search should present in other search engines section).
(3)remove any search engine(removing search engine should be top 5 in the list of other search engines) from the other search engine section and scroll down the page and observe.

Expected result: Only one focus should exist in the search engines page.

Actual Result: Two focus are available in the search engines page.

This is a regression issue broken in M-56

Manual bisect info:
=================
Good Build:56.0.2907.0
Bad Build:56.0.2908.0
 
Manage serach engines focus-1.ogv
5.2 MB View Download
Status: Untriaged (was: Unconfirmed)
unable to provide the chromium bisect as sign in doesn't work on chromium due to  Issue 667488 ,Providing manual change log.

Manual change log:
------------------
CL:https://chromium.googlesource.com/chromium/src/+log/56.0.2907.0..56.0.2908.0?pretty=fuller&n=10000

Able to reproduce the issue on Windows 10(non-corp) using Canary 59.0.3033.0.
Cc: dbeam@chromium.org dschuyler@chromium.org dpa...@chromium.org
Labels: -Needs-Bisect OS-Mac
Able to reproduce this issue on Mac OS 10.12.3 using chrome latest dev #58.0.3029.6.

While per-revision bisect unable to find the focus ring while deleting the search engine, due to this unable to find good and bad builds to find the actual suspect. Adding few dev person who have worked on MD settings under this CL for further triaging.

Thanks!


Comment 4 by dbeam@chromium.org, Mar 7 2017

Blocking: 671375
Cc: scottchen@chromium.org
Labels: -Pri-1 -M-58 Pri-2
Status: Available (was: Untriaged)

Comment 5 by dbeam@chromium.org, Mar 7 2017

Labels: Hotlist-MD-Settings-PageA11y
Components: -UI UI>Settings
Labels: Proj-MaterialDesign-WebUI
@scottchen: This seems like yet another iron-list bug where the a DOM element is reused to display different items of the list, WDYT?
dpapad@ that would also be my first suspect. Will pass it to the Polymer team and see what they think.
Are those two iron-list elements sharing the scrolling element?

Owner: scottchen@chromium.org
Status: Started (was: Available)
Status: Assigned (was: Started)
Cc: egarciad@chromium.org
egarciad@ If I understand your question correctly, they're both inside the same iron-list which has a scroll-target. See: https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/search_engines_page/search_engines_list.html?l=45

Some other discover while triaging that might be helpful to know:
- I was only able to reproduce this when I shrink my browser window down to a certain size, which I am guessing impacts how iron-list lazily loads.
- Upgrading to iron-list#1.4.4 did not solve this issue.

Comment 13 by dbeam@chromium.org, Mar 17 2017

Blocking: -671375
Update from keanulee@ on Polymer team:
This is happening because iron-list doesn't know that the removed item had focus on it, and just reuses its DOM node after it gets removed, which is why the focus is showing up in weird places. In other words, currently removing a focused item has an undefined behavior that might cause issues.

There's actually another bug caused by the same issue:
After you delete an item, even though the focus will appear to land on the "next item" that fills its spot, if you press up/down, the focus doesn't actually go where you'd expect, because the physical DOM array is already in a different order in respect to the index of the item that gets deleted.

The Polymer team is having a discussion on what behavior they should support, and their two options are:
- document.activeElement.blur() (i.e. focus nothing) and let the consumer decide what to do
- build in the functionality to find the correct "next item" and focus it.

In the meantime, we might have to explicitly manage the focus wherever we're removing stuff from iron-list. 
dbeam@ tbuckley@ now that we discovered keyboard behavior breaks as soon as something gets deleted from the list, should this be marked as a launch blocker?

Comment 16 by dbeam@chromium.org, Mar 31 2017

ehhhhh, i don't feel strongly either way.  we could probably survive with this on beta/stable
Status: Started (was: Assigned)
Project Member

Comment 18 by bugdroid1@chromium.org, Jun 6 2017

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

commit 3a7d55be3d2245f52c60b892c83ea17f62766b7c
Author: scottchen <scottchen@chromium.org>
Date: Tue Jun 06 18:53:34 2017

MD Settings: Fix iron-list losing focus when items change.

This CL makes a patch to the iron-list javascript code directly, in order to fix issues with iron-list focus bugs when the backing items changes.

BUG= 699001 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2794173002
Cr-Commit-Position: refs/heads/master@{#477356}

[modify] https://crrev.com/3a7d55be3d2245f52c60b892c83ea17f62766b7c/third_party/polymer/v1_0/chromium.patch
[modify] https://crrev.com/3a7d55be3d2245f52c60b892c83ea17f62766b7c/third_party/polymer/v1_0/components-chromium/iron-list/iron-list-extracted.js

Cc: jmukthavaram@chromium.org
 Issue 735899  has been merged into this issue.
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 23 2017

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

commit 20547cb0499f83a89e55c1cc222fffe4eac89a70
Author: scottchen <scottchen@chromium.org>
Date: Fri Jun 23 22:32:21 2017

MD Settings: add 'preserve-focus' flag to fix iron-list focus bugs.

This is a follow-up CL to a previous CL to patch iron-list (2794173002), in order to fix the issue of focus being lost after removing items.

BUG= 699001 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2949873003
Cr-Commit-Position: refs/heads/master@{#482051}

[modify] https://crrev.com/20547cb0499f83a89e55c1cc222fffe4eac89a70/chrome/browser/resources/settings/bluetooth_page/bluetooth_subpage.html
[modify] https://crrev.com/20547cb0499f83a89e55c1cc222fffe4eac89a70/chrome/browser/resources/settings/languages_page/edit_dictionary_page.html
[modify] https://crrev.com/20547cb0499f83a89e55c1cc222fffe4eac89a70/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
[modify] https://crrev.com/20547cb0499f83a89e55c1cc222fffe4eac89a70/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
[modify] https://crrev.com/20547cb0499f83a89e55c1cc222fffe4eac89a70/chrome/browser/resources/settings/search_engines_page/search_engines_list.html
[modify] https://crrev.com/20547cb0499f83a89e55c1cc222fffe4eac89a70/chrome/browser/resources/settings/search_engines_page/search_engines_page.html

Status: Fixed (was: Started)

Comment 22 by vku...@etouch.net, Jun 27 2017

Labels: TE-Verified-M61 TE-Verified-61.0.3141.0 TE-Verified-61.0.3142.0
Above issue seems to be fixed to Latest Dev Version:61.0.3141.0 (Official Build) 180095eb1bca7df1cdcb02547340499c2ee3af6e-refs/heads/master@{#482153} and Latest Canary Version:61.0.3142.0 (Official Build) using Windows (7,8,10),Mac Pro (10.12.3 & 10.11.6) and Linux (14.04 LTS)

Sign in to add a comment