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

Issue 704299 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Disconnect button in system menu VPN page opens VPN settings page

Project Member Reported by varkha@chromium.org, Mar 22 2017

Issue description

Chrome Version: M-58
OS: Chrome OS

User report:
When I click “Disconnect” from the tray for a VPN session, instead of disconnecting, I am not forced to go yet another screen and click Disconnect again.

Cause:
When MD "Disconnect" button was introduced we have lost a separate handler for it and the clicks go to the same handler as clicks elsewhere on the same row.
 
Labels: -M-59 M-58
I think this would be good to merge back into m-58.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 23 2017

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

commit c2637be18c2e52da142d6cbf646ab82045e13218
Author: varkha <varkha@chromium.org>
Date: Thu Mar 23 19:40:35 2017

[ash-md] Restores handler for VPN Disconnect button in system menu

The handler was unintentionally lost in MD work which got exposed after
turning the MD to be on by default in M-56.
Breaking CL - https://codereview.chromium.org/2463163002

BUG= 704299 

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

[modify] https://crrev.com/c2637be18c2e52da142d6cbf646ab82045e13218/ash/common/system/chromeos/network/vpn_list_view.cc

Comment 4 by varkha@chromium.org, Mar 23 2017

Labels: Merge-Request-58
The CL in #3 should be simple enough to get merged into M-58. It more or less restores the logic that existed earlier so should be relatively low risk.
Project Member

Comment 6 by sheriffbot@chromium.org, Mar 24 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 27 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/260f32670b26b1669f30ecf8d5009e78f1072d14

commit 260f32670b26b1669f30ecf8d5009e78f1072d14
Author: Valery Arkhangorodsky <varkha@chromium.org>
Date: Mon Mar 27 20:01:43 2017

[ash-md] Restores handler for VPN Disconnect button in system menu

The handler was unintentionally lost in MD work which got exposed after
turning the MD to be on by default in M-56.
Breaking CL - https://codereview.chromium.org/2463163002

BUG= 704299 

Review-Url: https://codereview.chromium.org/2768973002
Cr-Commit-Position: refs/heads/master@{#459182}
(cherry picked from commit c2637be18c2e52da142d6cbf646ab82045e13218)

Review-Url: https://codereview.chromium.org/2782473002 .
Cr-Commit-Position: refs/branch-heads/3029@{#439}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/260f32670b26b1669f30ecf8d5009e78f1072d14/ash/common/system/chromeos/network/vpn_list_view.cc

Comment 8 by varkha@chromium.org, Mar 27 2017

Components: UI>Shell>StatusArea
Status: Fixed (was: Started)
Labels: VPNsettings
Status: Verified (was: Fixed)
Works as expected. Tested on peppy R58-9334.72.0

Sign in to add a comment