Regression:ESC key does not work for Add language overlay. |
|||||||||
Issue descriptionChrome Version:60.0.3077.0 OS:Ububtu 14.04,Windows Steps to reproduce the problem: 1. Launch chrome and navigate to chrome://md-settings. 2. Go to Language and click on Add Languages. 3. Click on ESC key and observe(kindly refer video). What is the expected behavior? Add language overlay should gets closed on ESC key. What went wrong? Add language overlay is not getting closed on ESC key. Did this work before? Yes This is a regression issue broken in M-59. Manual Bisect Info: ==================== Good Build:59.0.3050.0 Bad Build:59.0.3051.3
,
Apr 21 2017
,
Apr 21 2017
Issue is seen on the latest canary: 60.0.3077.0 on Mac OS 10.12.3 as well.
,
Apr 21 2017
Using per-revision bisect providing bisect information below. Bisect Information: ----------------------- You are probably looking for a change made after 459337 (known good), but no later than 459338 (first known bad). Change Log URL: https://chromium.googlesource.com/chromium/src/+log/fed5bda99ca38b4e2575614c87e858f51b2a8194..9493bb90ea9b5cc4c5e91c388912d2cdc0cf7390 From the above change log suspecting below change Review URL: https://codereview.chromium.org/2772873002 dpapad@ - 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.
,
Apr 21 2017
I can reproduce this issue. When the focus is on the search input box 'Esc' does not close the dialog. Tabbing out of the search input and hitting 'Esc' still works. Tried a few of our other dialogs with text inputs, and 'Esc' is working even when the input is focused. Will investigate.
,
Apr 22 2017
After further investigation, this seems to be intentional. Pressing 'Esc' when the focus is within the search input is clearing the search query. @dbeam: Do you think that it is worth tweaking this dialog's logic to do the following? - Clear the search query if the query.length > 0 - Close the dialog if query.length == 0
,
Apr 22 2017
ehhhh, maybe. i kinda thought that was the issue. do we have other dialogs that focus an input by default? do those hide on Esc while focus is there?
,
Apr 22 2017
Yes we do, for example the add/edit startup URL dialog. The difference is that all our other dialogs that auto-focus an input, are not search inputs.
,
Apr 22 2017
i don't really love that when i mash down arrows or pagedown that when i reach the end of the scrollable area a new thing starts scrolling, but somebody somewhere thought that this is a good idea. i'm ok with either way: wontfix this bug and just say "Esc while focused in a search input just clears the text every time" or we could make "Esc with text clears it, Esc without text bubbles up and clears the dialog"
,
Apr 24 2017
,
Apr 24 2017
Slight preference for having Esc in an empty search box close the dialog.
,
Oct 24 2017
Marking bugs (mostly lower priority ones) that I am unlikely to get to soon as Available.
,
May 31 2018
,
Jun 6 2018
,
Jun 6 2018
@dbeam: Are you going for the "Esc with text clears it, Esc without text bubbles up and clears the dialog" approach?
,
Jun 6 2018
^ yes
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/298c086ca0619fc4bbbe91aeed9d695b16c4cdf0 commit 298c086ca0619fc4bbbe91aeed9d695b16c4cdf0 Author: dpapad <dpapad@chromium.org> Date: Thu Jun 14 19:40:12 2018 Settings WebUI: Close language dialog on esc, if no search query exists. Bug: 714021 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I3c96ce501e1d5ff65b8c27f7d04dab83694acc2e Reviewed-on: https://chromium-review.googlesource.com/1090000 Reviewed-by: Hector Carmona <hcarmona@chromium.org> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#567361} [modify] https://crrev.com/298c086ca0619fc4bbbe91aeed9d695b16c4cdf0/chrome/browser/resources/settings/languages_page/add_languages_dialog.html [modify] https://crrev.com/298c086ca0619fc4bbbe91aeed9d695b16c4cdf0/chrome/browser/resources/settings/languages_page/add_languages_dialog.js [modify] https://crrev.com/298c086ca0619fc4bbbe91aeed9d695b16c4cdf0/chrome/test/data/webui/settings/languages_page_tests.js
,
Jun 14 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by sahitya....@techmahindra.com
, Apr 21 2017