D26042: Captive portal notification improvements

2019-12-17 Thread David Edmundson
davidedmundson added a comment.


  Due to a confusing design notification objects can delete themselves.
  
  If the network changes after the notification is externally closed this will 
crash.

INLINE COMMENTS

> portalmonitor.h:42
> +private:
> +KNotification *m_notification = nullptr;
>  };

Needs to be QPointer

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D26042

To: jgrulich, ngraham, broulik, #plasma
Cc: davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D26042: Captive portal notification improvements

2019-12-17 Thread Jan Grulich
This revision was automatically updated to reflect the committed changes.
Closed by commit R116:1ffef126b8f0: Captive portal notification improvements 
(authored by jgrulich).

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26042?vs=71660=71706

REVISION DETAIL
  https://phabricator.kde.org/D26042

AFFECTED FILES
  kded/portalmonitor.cpp
  kded/portalmonitor.h

To: jgrulich, ngraham, broulik, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D26042: Captive portal notification improvements

2019-12-16 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Being in an airport right now myself (as is Harald, I imagine), I'm able to 
test this immediately. :) Works great, shipit, and sorry for introducing this 
regression when I made the notification persistent. I keep forgetting that its 
lifecycle needs to be managed in this case.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  captive-portal-improvements

REVISION DETAIL
  https://phabricator.kde.org/D26042

To: jgrulich, ngraham, broulik, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D26042: Captive portal notification improvements

2019-12-16 Thread Jan Grulich
jgrulich updated this revision to Diff 71660.
jgrulich added a comment.


  :  - Notification might be already closed

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26042?vs=71659=71660

BRANCH
  captive-portal-improvements

REVISION DETAIL
  https://phabricator.kde.org/D26042

AFFECTED FILES
  kded/portalmonitor.cpp
  kded/portalmonitor.h

To: jgrulich, ngraham, broulik, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D26042: Captive portal notification improvements

2019-12-16 Thread Jan Grulich
jgrulich created this revision.
jgrulich added reviewers: ngraham, broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
jgrulich requested review of this revision.

REVISION SUMMARY
  Do not duplicate captive portal notifications and close them when we are no 
longer
  behind captive portal.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  captive-portal-improvements

REVISION DETAIL
  https://phabricator.kde.org/D26042

AFFECTED FILES
  kded/portalmonitor.cpp
  kded/portalmonitor.h

To: jgrulich, ngraham, broulik
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart