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

Issue 612086 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----

Blocking:
issue 610941



Sign in to add a comment

Allow options.ConfirmDialog to also confirm transitions to false

Project Member Reported by dspaid@chromium.org, May 16 2016

Issue description

Feature description:
Currently options.ConfirmDialog (src/chrome/browser/resources/options/confirm_dialog.js) can only be used to show a confirmation when going from the disabled -> enabled state.  Modify it to also allow a dialog to be specified for the enabled -> disabled state change (opt-out).

Eng owner: dspaid@chromium.org
Product owner: mitsuji@chromium.org
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 25 2016

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

commit bf2c502f136f1f87a08d9a3546a893f6806b787a
Author: dspaid <dspaid@chromium.org>
Date: Wed May 25 02:13:42 2016

Allow ConfirmDialog to confirm disabling options.

BUG= 612086 
TEST=Manual testing. Preferences using this new functionality will
include their own tests.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/bf2c502f136f1f87a08d9a3546a893f6806b787a/chrome/browser/resources/options/confirm_dialog.js

Comment 2 by dspaid@chromium.org, Jul 13 2016

Status: Fixed (was: Assigned)

Comment 3 by ajha@chromium.org, Jul 28 2016

Cc: ajha@chromium.org
Labels: Needs-Feedback
dspaid@: If this needs verification, could you please provide any reduced test case & steps for verification.

Thank you!

Comment 4 by dspaid@chromium.org, Jul 31 2016

I think the verification for this can be done as part of the verification for the issue that depended on this change (610941) since that is the only test case currently.

Sign in to add a comment