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

Issue 714021 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression:ESC key does not work for Add language overlay.

Project Member Reported by racha.an...@techmahindra.com, Apr 21 2017

Issue description

Chrome 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
 
Actual_esc.ogv
2.1 MB View Download
Expected_esc.ogv
2.1 MB View Download
Description: Show this description
Cc: durga.behera@chromium.org kavvaru@chromium.org ajha@chromium.org brajkumar@chromium.org
Labels: -Pri-2 -Via-Wizard-UI Needs-Bisect M-60 OS-Windows Pri-1

Comment 3 by ajha@chromium.org, Apr 21 2017

Labels: -M-60 M-59 OS-Mac
Status: Untriaged (was: Unconfirmed)
Issue is seen on the latest canary: 60.0.3077.0 on Mac OS 10.12.3 as well.
Labels: -Needs-Bisect hasbisect-per-revision
Owner: dpa...@chromium.org
Status: Assigned (was: Untriaged)
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.

Comment 5 by dpa...@chromium.org, Apr 21 2017

Cc: dbeam@chromium.org
Components: -UI UI>Settings
Labels: -Pri-1 Proj-MaterialDesign-WebUI Pri-2
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.

Comment 6 by dpa...@chromium.org, 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

Comment 7 by dbeam@chromium.org, 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?

Comment 8 by dpa...@chromium.org, 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.

Comment 9 by dbeam@chromium.org, 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"
Labels: Hotlist-MD-Settings-Languages
Slight preference for having Esc in an empty search box close the dialog.
Owner: ----
Status: Available (was: Assigned)
Marking bugs (mostly lower priority ones) that I am unlikely to get to soon as Available.
Cc: nyerramilli@chromium.org rbasuvula@chromium.org
 Issue 848209  has been merged into this issue.
Owner: dbeam@chromium.org
Status: Started (was: Available)
@dbeam: Are you going for the "Esc with text clears it, Esc without text bubbles up and clears the dialog" approach?
^ yes
Project Member

Comment 17 by bugdroid1@chromium.org, 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

Owner: dpa...@chromium.org
Status: Fixed (was: Started)

Sign in to add a comment