| Summary: | Connection failed notification triggered despite successful connection | ||
|---|---|---|---|
| Product: | [Unmaintained] Network Management | Reporter: | Eike Hein <hein> |
| Component: | Wireless | Assignee: | Lamarque V. Souza <lamarque> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | adaptee, info, jgrulich, starbuck |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Fedora RPMs | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/networkmanagement/f5c57ab9ba95db528fa89906335f8dca44b43439 | Version Fixed In: | 0.9.0.9 |
| Sentry Crash Report: | |||
| Attachments: |
Suppress "connection failed" notification if connection is deleted
Partial backport of 4706c1c2 that applies cleanly to v0.9.0.8 Partial backport of 4706c1c2 and 6582b153 that applies cleanly to v0.9.0.8 |
||
|
Description
Eike Hein
2013-05-31 12:55:39 UTC
Sorry for the typo, s/KNotify connection/KNotify notification/ of course. not sure about kubuntu, but netrunner is reported to be affected as well (0.9.0.8-0ubuntu1) I remember seeing such notifications too, when playing with various recently released livecd for fun :) Created attachment 80219 [details]
Suppress "connection failed" notification if connection is deleted
Well, when the connection is deleted the failed connection notification should be triggered since the connection attempt really failed, but it should not wait until the next connection attempt to show up. I have tested your steps here and no notification appears, maybe because the object that triggers the notification is being deleted too fast. Can you recompile Plasma NM with the patch attached and see if it suppress the "connection failed" notification?
The patch doesn't appear to help, I'm still getting the notification. Note that since I was evaluating for distro inclusion I tested it against the v0.9.0.8 tag, not master. (Also curious btw: In order for clicking a WLAN list item to bring up the dialog, I had to install into /usr, a folder in $KDEDIRS/$PATH in my $HOME didn't work as it normally does with KDE apps.) Addendum: In addition to the notification I'm also getting a failed tray icon now, briefly, before it is replaced with the success one. I believe (but am not entirely sure) that's new with the patch, before it went straight to the success icon. Hm, but not always either @ icon, so it may have been happening before, but is simply quite racy. Sorry for the reply noise. By the way, you have to restart kded4 (or relogin) for the patch to take effect. Are you really sure you installed it in /usr? If you install it in another path probably Plasma NM's libraries in /usr will take precedence and the patch will not take effect. The systray icon glitch is unrelated to this patch, the notification system and the tray icon are handled by two different system proccesses (kded4 and plasma-desktop). Just for debugging, does the notification appears if you disable all Plasma NM's notifications in "Manage Connections" -> "Other" -> "Configure Notifications"? - I logged in and out. - I tried removing the system version of KNM completely, but still had to install to /usr for the dialog to work, so maybe the KNM code uses KStandardDirs incorrectly somewhere (installing to $KDEDIRS and even overriding a system copy that way is supposed to work). I'll have a look at the notification thing later tonight, can't cut my connection at the moment. Yes, the notification isn't triggered if I disable it in the KNotify settings. FWIW, of the two possible KNotification callsites for this message, the one taken is notificationmanager.cpp:129. So that means there's a state transition from Activating to Unknown that it declares as connection failure, which is apparently an incorrect assumption to make. Can you test if the commit below fixes this problem? http://commits.kde.org/networkmanagement/4be000f0b162c52d4c7e0ffdc2f0143b1cfffcdb To test it just 'git pull' either master or nm09 branches and recompile it. I'll test, but can you comment on whether you expect this patch to work against 0.9.0.8? If it works, I intend to ship this in a distro where this bug is considered a release blocker candidate (a confusing error message on initial connect is kind of terrible for users). The patch requires NetworkManager 0.9.8, if the distribution you mentioned does not use that NetworkManager version then you will have to upgrade. This patch applies to Plasma NM 0.9.0.8 and you can even compile Plasma NM against NetworkManager 0.9.8 and then run a lower NetworkManager version but then the patch will have no effect since the notification manager now depends on the two new connection states (deactivating and deactivated) that only NetworkManager >= 0.9.8 uses. I am about to test this with a simplified patch applied to v0.9.0.8, but please note that modern compilers will (with good reason) warn about this code because the switch statement in notificationmanager.cpp doesn't handle the Deactivating (as opposed to Deactivated) value and it has no default case. Created attachment 80529 [details]
Partial backport of 4706c1c2 that applies cleanly to v0.9.0.8
Yay - the attached patch that applies cleanly to v0.9.0.8 no longer incorrectly announces connection failure.
I'll hold off on shipping this for now however in case you want to have another crack at that switch statement first?
Git commit 6582b1534e51e959593b17c8d4e52aa494e1d28c by Lamarque V. Souza. Committed on 15/06/2013 at 16:04. Pushed by lvsouza into branch 'nm09'. Distinguish between user deactivation and activation error for vpn connections as well. FIXED-IN: 0.9.0.9 (cherry picked from commit f5c57ab9ba95db528fa89906335f8dca44b43439) M +11 -4 libs/service/notificationmanager.cpp http://commits.kde.org/networkmanagement/6582b1534e51e959593b17c8d4e52aa494e1d28c Git commit f5c57ab9ba95db528fa89906335f8dca44b43439 by Lamarque V. Souza. Committed on 15/06/2013 at 16:04. Pushed by lvsouza into branch 'master'. Distinguish between user deactivation and activation error for vpn connections as well. FIXED-IN: 0.9.0.9 M +11 -4 libs/service/notificationmanager.cpp http://commits.kde.org/networkmanagement/f5c57ab9ba95db528fa89906335f8dca44b43439 Created attachment 80552 [details]
Partial backport of 4706c1c2 and 6582b153 that applies cleanly to v0.9.0.8
Thanks Lamarque; attached is the patch we'll apply to v0.9.0.8.
*** Bug 322631 has been marked as a duplicate of this bug. *** How do I Install this on Ubuntu 13.04. is there a .deb package available. How do I Install this on Ubuntu 13.04. is there a .deb package available. |