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

Issue 736293 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unnecessary focus traversal to background page is seen in Confirm Powerwash overlay.

Project Member Reported by jbanavatu@chromium.org, Jun 23 2017

Issue description

Chrome Version: 61.0.3138.0/9677.0.0 dev channel Paine,Daisy,Candy
OS: Chrome OS

What steps will reproduce the problem?
(1)Sign in to chrome >> Go to chrome://settings/reset >> Select powerwash option and click on restart button in 'Restart your device' overlay  
(2)Go to 'Confirm Powerwash screen by clicking powerwash button in 'Reset this Chrome Device' screen 
(3)Now keep on hitting tab and Observe focus traversal

Expected: Focus should not traverse to options in background page.
Actual: Instead, Unnecessary focus traversal to background page is seen.

This is regression issue as no such behavior is seen in M58.

Attaching screen-cast for reference.

Note: Issue is not applicable to Linux and Windows OS.
 
Actual.mp4
5.7 MB View Download
Owner: alemate@chromium.org
Status: Assigned (was: Untriaged)
Alex, please take a look. Thank you!

Comment 2 by r...@chromium.org, Jun 23 2017

Labels: -M-61 M-62

Comment 3 by kochi@chromium.org, Jun 26 2017

I'm not familiar with Chrome OS UI, but is this based on WebUI, which
uses <dialog> element and Blink's focus system?
Cc: alemate@chromium.org
Owner: dpa...@chromium.org
Assigning to dpapad@ as this is not OOBE/Signin UI.

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

Components: -Blink>Focus UI>Shell>OOBE
Owner: ----
Status: Available (was: Assigned)
@alemate: If it was a <dialog> it would not have this problem. Looking at the code it is using an oobe-dialog, which is not based on native <dialog> or our own MD-looking <cr-dialog> [2].

My suggestion is to migrate OOBE code to use cr-dialog. There would be one less dialog implementation, and the focus issue would be automatically fixed.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/oobe_dialog.html?q=oobe_dialog.html&sq=package:chromium&dr&l=55
[2] https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html

Comment 6 by dpa...@chromium.org, Jul 21 2017

Cc: dpa...@chromium.org
Components: -UI -UI>Shell>OOBE UI>Settings
As I understand the description correctly, the issue is basically about 'Confirm Powerwash' not being modal.
I don't see any issues with the OOBE dialog. Focus is working correctly with OOBE dialogs itself, I suppose.

Comment 9 by dpa...@chromium.org, Jul 21 2017

The dialog seems to be partially modal. User can't access anything behind it with the mouse, but can with the keyboard.

These are the same problems we faced in MD Settings before we switch to a native <dialog> (we used paper-dialog initially), see  issue 625332  and  issue 720781 . There was a lot of discussion/research that led us to this decision.

There is no reason not to use native modal <dialog> when writing Chrome-only code. If you cared about cross-browser compatibility it would be a different discussion.

Comment 10 Deleted

I was wrong, I thought that is the same popup that is used in settings, but it is not. I'll look into it.
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 25 2017

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

commit 25cd71890c8c28fa45eabc6265e40c4a038864b0
Author: Alexander Alekseev <alemate@chromium.org>
Date: Fri Aug 25 00:33:17 2017

Chrome OS: Make Powerwash dialog overlay modal.

This Cl makes Chrome OS Powerwash dialog modal.

Bug:  736293 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I2dd8dbb5990907a8c959ee826db2545d240dac32
Reviewed-on: https://chromium-review.googlesource.com/634265
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497261}
[modify] https://crrev.com/25cd71890c8c28fa45eabc6265e40c4a038864b0/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.css
[modify] https://crrev.com/25cd71890c8c28fa45eabc6265e40c4a038864b0/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.html
[modify] https://crrev.com/25cd71890c8c28fa45eabc6265e40c4a038864b0/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.js
[modify] https://crrev.com/25cd71890c8c28fa45eabc6265e40c4a038864b0/chrome/browser/resources/chromeos/login/oobe_screen_reset.js

Status: Fixed (was: Available)

Sign in to add a comment