> 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 > >