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

Issue metadata

Status: Verified
Owner:
Closed: Aug 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 137941: Wifi network's showing up as VPN's

Reported by bmahadev@chromium.org, Jul 18 2012 Project Member

Issue description

Chrome Version: 21.0.1180.49
Chrome OS Version: 2465.75.0
Chrome OS Platform: x86

Steps To Reproduce:
1.Login to the device and configure few vpn's
2.Try to alternately reconnect to either of them
3.During this process if the vpn connection fails for some reason or if you force failure through say, giving wrong password or incorrect certificate, check the wifi dropdown

Expected Result:
Wifi networks should only have wifi badges

Actual Result:
Some or all wifi's show up with vpn badges

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)
Sometimes

What is the impact to the user, and is there a workaround? If so, what is
it?
Incorrect network representation

Please provide any additional information below. Attach a screen shot or
log if possible.
 

Comment 1 by bmahadev@chromium.org, Jul 18 2012

Logs - http://cros-hwqual-5.mtv.corp.google.com/bugs/137941/
screenshot1.png
281 KB View Download

Comment 2 by stanleyw@chromium.org, Jul 18 2012

Status:

Comment 3 by stanleyw@chromium.org, Jul 18 2012

Status: Available
It looks like the wifi icon is inconsistent throughout the list some show the 'vpn' symbol, other shows a normal wifi, etc.  Eg.  iptime (an open wifi network) vs Google-A.

Comment 4 by bmahadev@chromium.org, Jul 19 2012

The above issue can be easily reproduced by forcing a vpn connection failure.

Noticed another scenario where all the wifi's show up with vpn badges(attached) if we are previously connected to a vpn and the subsequent vpn connection to a different vpn fails.
1.Successfully connect to vpn(say l2tp/sec)
2.Now try connecting to a different vpn(openswan) and force it to fail. We then hit the second scenario.

List services o/p - http://cros-hwqual-5.mtv.corp.google.com/bugs/137941/list-servives.out
screenshot2.png
282 KB View Download

Comment 5 by krisr@chromium.org, Jul 19 2012

Labels: -Area-Undefined Area-UI
Status: Untriaged

Comment 6 by saintlou@chromium.org, Jul 19 2012

Owner: kuscher@chromium.org
Status: Assigned
Alex?

Comment 7 by kuscher@chromium.org, Jul 19 2012

Cc: bmahadev@chromium.org
Labels: Feature-Options Feature-Ash
Owner: kevers@chromium.org
@Kevin, is that something from you?

@bmahadev@, does this also happen in

Comment 8 by bmahadev@chromium.org, Jul 19 2012

I saw this happening on my lumpy device.

Comment 9 by bmahadev@chromium.org, Jul 19 2012

Issue also reproduces on 2628.0.0,R22. This time on zgb.

Comment 10 by kuscher@chromium.org, Jul 19 2012

Sorry, my sentence was incomplete .. does this reproduce in the status area network part (bottom right)

Comment 11 by kevers@chromium.org, Jul 20 2012

Cc: steve...@chromium.org
Looks like a problem in network_menu_icon.cc.  At the bottom of the SetBadges method, the VPN badge is added if connected or in the process of connecting to a virtual network.  The Update method tests to see of the cached icon requires regeneration.  Looks like there is no test to see if the VPN changes state in order to force a regeneration of the network icon.

Don't have easy access to VPN over WiFi, so it will be difficult to properly test a fix.  Steven, can you take a look at this?

Comment 12 by kevers@chromium.org, Jul 27 2012

Cc: kevers@chromium.org
Owner: ----
Status: Available

Comment 13 by kuscher@chromium.org, Jul 27 2012

Labels: Mstone-22
Owner: steve...@chromium.org
Status: Assigned

Comment 14 by saintlou@chromium.org, Aug 6 2012

Labels: Iteration-62
All Pri-1 for M22 -> Iteration-62

Comment 15 by steve...@chromium.org, Aug 6 2012

Status: Started

Comment 16 by steve...@chromium.org, Aug 7 2012

There were a few bugs, and some confusing UI. I made the following changes:
* Moved the ShouldShowIconInTray() logic into NetworkMenuIcon.
* Removed the unused GetVpnImage()
* Made VPN icon code is now similar to other network types
* Changed network display order to:
** Active network
** Connected networks (VPN, Ethernet, Cellular, Wimax, Wifi)
** Unconnected networks (same order)

Comment 18 by steve...@chromium.org, Aug 8 2012

Labels: Merge-Requested

Comment 19 by bugdroid1@chromium.org, Aug 8 2012

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=150607

------------------------------------------------------------------------
r150607 | stevenjb@google.com | 2012-08-08T20:34:41.898766Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chromeos/status/network_menu_icon.cc?r1=150607&r2=150606&pathrev=150607
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chromeos/status/network_menu_icon.h?r1=150607&r2=150606&pathrev=150607
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chromeos/system/ash_system_tray_delegate.cc?r1=150607&r2=150606&pathrev=150607
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chromeos/cros/network_library_impl_stub.cc?r1=150607&r2=150606&pathrev=150607

Fix logic for VPN badges and network menu order

There were a few bugs, and some confusing UI. I made the following changes:
* Moved the ShouldShowIconInTray() logic into NetworkMenuIcon.
* Removed the unused function GetVpnImage()
* Made VPN icon code similar to other network types
* Changed network display order to:
** Active network
** Connected networks (VPN, Ethernet, Cellular, Wimax, Wifi)
** Unconnected networks (same order)

BUG= 137941 

Review URL: https://chromiumcodereview.appspot.com/10824208
------------------------------------------------------------------------

Comment 20 by k...@google.com, Aug 8 2012

Labels: -Merge-Requested Merge-Approved

Comment 21 by bugdroid1@chromium.org, Aug 8 2012

Project Member
Labels: -Merge-Approved merge-merged-1229
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=150620

------------------------------------------------------------------------
r150620 | stevenjb@google.com | 2012-08-08T21:55:46.464453Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1229/src/chrome/browser/chromeos/status/network_menu_icon.cc?r1=150620&r2=150619&pathrev=150620
   M http://src.chromium.org/viewvc/chrome/branches/1229/src/chrome/browser/chromeos/status/network_menu_icon.h?r1=150620&r2=150619&pathrev=150620
   M http://src.chromium.org/viewvc/chrome/branches/1229/src/chrome/browser/chromeos/system/ash_system_tray_delegate.cc?r1=150620&r2=150619&pathrev=150620
   M http://src.chromium.org/viewvc/chrome/branches/1229/src/chrome/browser/chromeos/cros/network_library_impl_stub.cc?r1=150620&r2=150619&pathrev=150620

Merge 150607 - Fix logic for VPN badges and network menu order

There were a few bugs, and some confusing UI. I made the following changes:
* Moved the ShouldShowIconInTray() logic into NetworkMenuIcon.
* Removed the unused function GetVpnImage()
* Made VPN icon code similar to other network types
* Changed network display order to:
** Active network
** Connected networks (VPN, Ethernet, Cellular, Wimax, Wifi)
** Unconnected networks (same order)

BUG= 137941 

Review URL: https://chromiumcodereview.appspot.com/10824208

TBR=stevenjb@google.com
Review URL: https://chromiumcodereview.appspot.com/10829249
------------------------------------------------------------------------

Comment 22 by steve...@chromium.org, Aug 8 2012

Status: Fixed
Merged to 1229 @ 150620

Comment 23 by bhaveshmm@chromium.org, Aug 10 2012

Labels: Network

Comment 24 by bgadhavi@chromium.org, Aug 13 2012

Status: Verified
Tested and verified on: 
Chrome Version: 22.0.1229.4
Chrome OS Version: 2723.13.0

Comment 25 by bmahadev@chromium.org, Aug 14 2012

Status: Assigned
Re-opening as the issue is still not fixed on R21. Tested on 2465.121.0,R21.

Comment 26 by steve...@chromium.org, Aug 14 2012

Cc: kuscher@chromium.org
This is a non trivial patch, do we want to merge it into M21? It only affects VPNs and is only cosmetic (albeit confusing), i.e. it doesn't affect functionality at all.

Comment 27 by saintlou@chromium.org, Aug 14 2012

Status: Fixed
I agree with Steven, why do we need this so badly in M21?

Comment 28 by kuscher@chromium.org, Aug 14 2012

Agreed, this is fine in 22/23 - not worth risking the stability I think.

Comment 29 by bmahadev@chromium.org, Aug 15 2012

Status: Verified
Marking verified as per #24.

Comment 30 by bugdroid1@chromium.org, Oct 13 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 31 by lafo...@chromium.org, Oct 15 2012

Labels: -Feature-Options Feature-Settings

Comment 32 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-UI -Feature-Ash -Mstone-22 -Feature-Settings Cr-UI-Settings Cr-UI-Shell Cr-UI M-22

Comment 33 by bugdroid1@chromium.org, Mar 14 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment