Regression:Focus does not stay on iron icon after deleting entry from history page. |
||
Issue descriptionChrome Version:73.0.3633.0 (Official Build) c972e8079a1fb95f6cb7c3afc162ea473384c3b1-refs/branch-heads/3633@{#1}(32/64 bit) OS: Windows(7,8,8.1,10), Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04) Pre-condition : Atleast two entries should be present under chrome://history What steps will reproduce the problem? (1) Launch Chrome navigate to chrome://history. (2) Press tab until focus reaches to Actions icon for an entry and click on it. (3) Now press tab key to bring focus on 'Remove from History' option and press enter key. (4) Observe. Actual Result :Focus does not stay on iron icon after deleting entry from history page. Expected Result: Focus should stay on iron icon even after deleting entry from history page. This is regression issue broken in ‘M-68’, Good build: 68.0.3400.0(551876) Bad build : 68.0.3401.0(552221) (Unable to provide bisect using Per-revision script as Traceback error message is thrown. Also, tried bisecting on other machines and same error is thrown. Hence, providing suspect using Chromium bisect) Chromium Bisect info: https://chromium.googlesource.com/chromium/src/+log/e83aa862864a77b3f2a9109c92efed33943e98c9..9f7279479f4be59322d1e6146afe9200282887c5 Suspect: https://chromium.googlesource.com/chromium/src/+/9417cbbdf2bf0fb3b82cc20e0b0e5cd808d71b16 @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. Thank You!
,
Dec 7
,
Dec 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8590a077110d1cc40a70fe053f76f0056569027c commit 8590a077110d1cc40a70fe053f76f0056569027c Author: rbpotter <rbpotter@chromium.org> Date: Sat Dec 08 03:32:33 2018 History/Settings: Maintain focus position when items are removed Removing items results in focus leaving the list and then being restored directly to a list item, because when preserve-focus is set iron-list calls blur() on the currently focused element and then focuses the next item in the list. Normally, we want to reset the focus position if focus leaves the list and is then restored to a list item, as this occurs when the user blurs the list and then tabs back into the list from the beginning. The difference between these cases can be detected by watching ironListTabIndex, which changes when items are added to or removed from the list. Clearing the listBlurred parameter in this case means that FocusRowBehavior will treat the subsequent focus event as if focus had never left the list, which is the desired behavior. Also fixing some runtime errors observed on chrome://history, that are likely due to Polymer 2 allowing observers to be called with undefined arguments. These errors seemed to show up consistently with this CL in debug builds, so it appears the change altered the timing slightly and triggers the observers being called before their arguments are defined. Bug: 911444 , 912926, 912900 Change-Id: Ib1a04ed0da6650d2f5e24787e36f4247c0364d51 Reviewed-on: https://chromium-review.googlesource.com/c/1368782 Reviewed-by: Scott Chen <scottchen@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/master@{#614935} [modify] https://crrev.com/8590a077110d1cc40a70fe053f76f0056569027c/chrome/browser/resources/md_history/history_item.js [modify] https://crrev.com/8590a077110d1cc40a70fe053f76f0056569027c/chrome/browser/resources/md_history/history_list.js [modify] https://crrev.com/8590a077110d1cc40a70fe053f76f0056569027c/ui/webui/resources/js/cr/ui/focus_row_behavior.js
,
Dec 10
Update: Rechecked the above issue on Windows(7,8,8.1,10), Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04) using latest canary build #73.0.3636.0 and issue is still reproducible.After deleting entry from chrome://history ,focus is not seen on next iron icon in list. please refer attached screencast. Thank You..
,
Dec 11
This is still happening (flakily, cannot consistently reproduce on Linux at least) because deletion seems to often trigger a new queryHistory call, which results in the list being cleared and reloaded. Stepping through in dev tools shows the list actually completely disappearing at one point. Obviously focus cannot be retained in this case, since there are no items to focus when the list is cleared. Still trying to determine why this is occurring, and why it does not seem to happen on debug builds, but it does not appear to be a new issue.
,
Dec 11
Tracked this to BrowsingHistoryService::OnURLsDeleted, see https://cs.chromium.org/chromium/src/components/history/core/browser/browsing_history_service.cc?l=745. Sometimes this ends up calling HistoryDeleted(), which has the effect described above, and other times it does not. The DeleteHistory() call happens if the backend detects more items that need to be deleted than were requested in the UI, which occurs when visits and redirects here: https://cs.chromium.org/chromium/src/components/history/core/browser/expire_history_backend.cc?l=283 are more numerous than the number of visits. Note that this can happen even if there is only a single entry in the UI for the redirects and visits, so only one item appears to be deleted. |
||
►
Sign in to add a comment |
||
Comment 1 by scottchen@chromium.org
, Dec 7