Re: Review Request 128707: Add support for captive portals

Kai Uwe Broulik <[email protected]>
Newsgroups gmane.comp.kde.knetworkmanager,gmane.comp.kde.usability,gmane.comp.kde.devel.plasma
Message-ID <[email protected]>
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128707/#review99161
-----------------------------------------------------------


Fix it, then Ship it!




Make sure to update your commit message to reflect the new way this patch works


kded/networkmanagement.notifyrc (line 779)
<https://git.reviewboard.kde.org/r/128707/#comment66763>

    Also a tip. If you just set the IconName properly here, you don't need to explicitly set it programmatically later ;)



kded/portalmonitor.h (line 28)
<https://git.reviewboard.kde.org/r/128707/#comment66764>

    Unused



kded/portalmonitor.cpp (line 55)
<https://git.reviewboard.kde.org/r/128707/#comment66765>

    You're still using setPixmap here (and then you can also remove KIconLoader include)


- Kai Uwe Broulik


On Sept. 14, 2016, 9:02 vorm., Jan Grulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128707/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2016, 9:02 vorm.)
> 
> 
> Review request for Network Management, Plasma, KDE Usability, and Lamarque Souza.
> 
> 
> Bugs: 365417
>     http://bugs.kde.org/show_bug.cgi?id=365417
> 
> 
> Repository: plasma-nm
> 
> 
> Description
> -------
> 
> Adds portal monitor to our kded module, which checks NetworkManager connectivity. If the value gets changed to NM_CONNECTIVITY_PORTAL (means we are behind a captive portal), then we open a QWebEngineView trying to load "http://kde.org" page which is supposed to be redirected to the captive portal page. Once user logs in and url changes, we re-check the connectivity again and close the web view if we are no longer behind the captive portal.
> 
> 
> Diffs
> -----
> 
>   kded/CMakeLists.txt 1f0613e 
>   kded/networkmanagement.notifyrc f95d94b 
>   kded/portalmonitor.h PRE-CREATION 
>   kded/portalmonitor.cpp PRE-CREATION 
>   kded/service.cpp 18ffd41 
> 
> Diff: https://git.reviewboard.kde.org/r/128707/diff/
> 
> 
> Testing
> -------
> 
> Tested with three different captive portals and it worked perfectly.
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.