New issue
Advanced search Search tips

Issue 590960 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

SSL Client cert dialog UX is inconsistent

Project Member Reported by est...@chromium.org, Mar 1 2016

Issue description

repro link: https://www.scriptjunkie.us/auth/verifycert

When you close the dialog with Escape or pressing Cancel, it proceeds with an empty cert (and saves that choice for the future).

When you close the dialog by clicking the (x) button at the top right, it aborts the request.

To my mind, the difference between escape and the close button makes no sense. Most dialogs will do the same thing because they don't override DialogDelegate::Close(), which defaults to calling Cancel(). But some dialogs secretly do handle (x) and ESC differently --- the user has no way of knowing except by reading Chrome's source code.
 
Note this is quite like bug 467155 and any fix to one of these should be tested against the other.
Status: Fixed (was: Untriaged)
This should have been fixed by:

https://codereview.chromium.org/1953943003

estade: Feel free to reopen if you think otherwise.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 6 2016

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

commit aceb9ee10952068609a2859e2e54205dc593535f
Author: xhwang <xhwang@chromium.org>
Date: Thu Oct 06 23:47:10 2016

Update comments on pressing Esc on PlatformVerificationDialog

On the latest build pressing Esc on PlatformVerificationDialog will call
Close() instead of Cancel(). So it seems issue 467155 is fixed. Updating
comments to reflect the current state.

Also update comments in DialogDelegate.

TBR=dkrahn@chromium.org
BUG=467155, 590960 
TEST=No functionality change.

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

[modify] https://crrev.com/aceb9ee10952068609a2859e2e54205dc593535f/chrome/browser/chromeos/attestation/platform_verification_dialog.cc
[modify] https://crrev.com/aceb9ee10952068609a2859e2e54205dc593535f/chrome/browser/media/protected_media_identifier_permission_context.cc
[modify] https://crrev.com/aceb9ee10952068609a2859e2e54205dc593535f/ui/views/window/dialog_delegate.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aceb9ee10952068609a2859e2e54205dc593535f

commit aceb9ee10952068609a2859e2e54205dc593535f
Author: xhwang <xhwang@chromium.org>
Date: Thu Oct 06 23:47:10 2016

Update comments on pressing Esc on PlatformVerificationDialog

On the latest build pressing Esc on PlatformVerificationDialog will call
Close() instead of Cancel(). So it seems issue 467155 is fixed. Updating
comments to reflect the current state.

Also update comments in DialogDelegate.

TBR=dkrahn@chromium.org
BUG=467155, 590960 
TEST=No functionality change.

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

[modify] https://crrev.com/aceb9ee10952068609a2859e2e54205dc593535f/chrome/browser/chromeos/attestation/platform_verification_dialog.cc
[modify] https://crrev.com/aceb9ee10952068609a2859e2e54205dc593535f/chrome/browser/media/protected_media_identifier_permission_context.cc
[modify] https://crrev.com/aceb9ee10952068609a2859e2e54205dc593535f/ui/views/window/dialog_delegate.h

Comment 5 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment