[Sugar-devel] Proxy Settings in Network Control Panel
ajay at activitycentral.com
Thu Mar 14 00:43:15 EDT 2013
Thanks a ton Manuel; I will test the patch on the weekend, and let you know
On Thu, Mar 14, 2013 at 7:27 AM, Manuel Quiñones <manuq at laptop.org> wrote:
> Hi Ajay and other devs,
> Ajay, whlie reviewing your patch for the proxy feature, I decided to
> step in and work on the issues I've found by myself. I did this to
> speed up the process, instead of going through a long
> review/patch/review loop. I hope you understand. A (rather long,
> sorry) description of what I did follows. But first of all, I would
> be glad if you can test my last patch:
> The first thing I've found is that your patch does both GConf and
> GSettings. I decided to go just for GSettings because Browse and all
> underlayng GTK use it.
If we are sure that no application carries the legacy "Gconf"
proxy-settings/schema, we are done, (not to forget less lines of code to
> In any case I think you should have added
> another mixin class for GSettings instead of adding code to GConfMixin
> class. Sasha's original patch had that quite separated to make adding
> another backend an easy task.
> So my first attempt was doing only GSettings and fixing the issues
> I've found in yours:
> (note this is not my final patch, let's call this my a) patch)
> An issue with your patch: the UI does not always update when a setting
> is changed. TestCase: change a setting, quickly close the network
> section and open it again: you will see the original value. This is
> because the timeout that calls _commit has not finished yet. Sasha's
> patch had gconf client.notify_add() for this. In my a) patch I have
> added a callback to the 'changed' event of gsettings that does the
I had fixed this, in the patch I had floated :)
The "self._section_view.perform_accept_actions()" did the fix in my patch;
now substituted by "self._section_view.apply()" in your patch.
I had even mentioned that in point c) in the patch at
Anyways, the end result is that, we HAVE TAKEN care of the racy-behaviour,
as far as the persistence of values is concerned.
> There are other differences between Sasha's patch and yours. For
> example you seem to forgot the try/finally clause in the _commit
> method that blocks the __changed_cb callback. I have added it back in
> my a) patch.
> I intentionally skipped $http_proxy environment variable (Sasha's patch
> This could be added later if there is a reason. I don't see GNOME
> doing it. Which activities or parts of Sugar need it? Indeed this
> makes wget work with proxy by default but then why GNOME does not do
> it? Am I missing any conversation?
Hmm.. One place where I know for confirmed it is used in "webkitgtk3", thus
making its use in "Browse".
But you said earlier that using ONLY GSettings (without setting the
"http_poxy" variable), propogates the proxy-settings in "Browse". If that
is the case, I think setting "http_proxy" is not needed.
For more details of the "http_proxy" interactions with GTK+, please see the
source-code of webkit, in particular
> So, now for my final patch. After having my a) patch working, I took
> another path using the convenient way GSettings provides to bind
> widgets and settings:
> The adventages can be seen in the ~100 lines removed. There is...
> - no need for a mixin class,
> - no need for connecting callbacks to signals between widgets and
> settings in both directions
> - no need to implement get_value_for_gsettings /
> set_value_from_gsettings for each widget. Sasha's gconf
> implementation needed a big hierarchy of classes to archive this.
> - no need to test the key type to map the corresponding getter ('s' to
> use get_string(), for example, compare with my a) patch)
These look good !!!
> So, I welcome testing in different proxy scenarios for my patch.
> Thanks a lot Ajay for your contributed work, which I took as basement.
> .. manuq ..
Activity Central: http://activitycentral.com
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Sugar-devel