> On Nov. 28, 2013, 5:17 a.m., Dawit Alemayehu wrote:
> > konqueror/settings/kio/kproxydlg.cpp, line 295
> > <http://git.reviewboard.kde.org/r/114105/diff/1/?file=219633#file219633line295>
> >
> >     nitpick: no need for KSaveIOConfig here.

?. Not sure what do you mean


> On Nov. 28, 2013, 5:17 a.m., Dawit Alemayehu wrote:
> > konqueror/settings/kio/kproxydlg.cpp, line 444
> > <http://git.reviewboard.kde.org/r/114105/diff/1/?file=219633#file219633line444>
> >
> >     This does not make sense. I explicitly check show me the value and you 
> > uncheck it as a result of me clicking on the "Auto Detect" button? No.

This is my (user) point of view:
if i'm configuring "system proxy" and i click "Auto Detect" button i'm 
interested to know/see what env vars i'm actually going to use,
not really their values. If i'm a curious user i could ask the gui to show me 
their *actual* values
(those values can also not be the same later on, for example because i 
re-assigned the env vars being configured).

This change is also a fast and simple fix for the following use case:
suppose i have set the following env vars on my system
$export my_proxy=http://localhost:1111
$export http_proxy=http://localhost:2222
and i have configured "system proxy" to use my_proxy (in kioslaverc 
httpProxy=my_proxy).
I open proxy config module and check the "Show the value of the environment 
variables" checkbox,
"http://localhost:1111"; is showed in "HTTP Proxy:",
then i push "Auto Detect",
"HTTP Proxy:" changes to "http://localhost:2222";,
i save and exit: nothing is really saved because of the logic adopted in 
KProxyDialog::save().


- Andrea


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114105/#review44646
-----------------------------------------------------------


On Nov. 27, 2013, 9:22 p.m., Andrea Iacovitti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114105/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2013, 9:22 p.m.)
> 
> 
> Review request for KDE Base Apps and Dawit Alemayehu.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> In case of 'system proxy' proxyType, NoProxyFor config key holds the name of 
> the env variable (e.g. no_proxy) and not its value.
> Because of this KProtocolManager::noProxyFor() can not be used to get 
> NoProxyFor config setting in KProxyDialog::load(), as it returns the resolved 
> value of the environment variable and not its name: i added helper method 
> KSaveIOConfig::noProxyFor() to read that value directly from config file.
> Also make sure to uncheck showEnvValueCheckBox before filling proxy edit 
> fields with environment variable names in 
> KProxyDialog::on_autoDetectButton_clicked().
> 
> 
> Diffs
> -----
> 
>   konqueror/settings/kio/kproxydlg.cpp e80afeb 
>   konqueror/settings/kio/ksaveioconfig.h 2318198 
>   konqueror/settings/kio/ksaveioconfig.cpp c822f7b 
> 
> Diff: http://git.reviewboard.kde.org/r/114105/diff/
> 
> 
> Testing
> -------
> 
> To reproduce the issue:
> $ export no_proxy=kde.org
> $ kcmshell4 proxy
> choose "Use system proxy configuration", push "Auto Detect" button, close the 
> gui interface and reopen it:
> $ kcmshell4 proxy
> see how Exceptions fields contains "kde.org" and not "no_proxy"
> 
> 
> Thanks,
> 
> Andrea Iacovitti
> 
>

Reply via email to