Re: [Tails-dev] Please review&merge feature/torbutton-1.5.x

Delete this message

Reply to this message
Author: Alan
Date:  
To: tails-dev
Subject: Re: [Tails-dev] Please review&merge feature/torbutton-1.5.x
Hi,

On Mon, 22 Apr 2013 21:36:12 +0200 sajolida@??? wrote:
> On 19/04/13 01:36, intrigeri wrote:
> > Hi,
> >
> > branch: feature/torbutton-1.5.x
> > ticket: https://tails.boum.org/todo/Torbutton_1.5/
> > Candidate for 0.18.
> >
> > This pull request is a bit less polished than I would like, but I'd
> > really like Torbutton 1.5.1 to go in the next Tails release, and
> > I've got to go, so here it is.
> >
> > I've put that branch, a bit in a hurry, in hopefully good enough
> > shape. All major issues I was able to detect were solved. There are
> > a few things left to check (see ticket), though -- I'm now quite
> > confident they should be alright. Fingers crossed.
> >
> > Bonus:
> >
> >   * This branch reorganizes and cleans our iceweasel prefs to look
> >     a bit more like TBB's (no behavior change, just easier to
> >     understand, compare and maintain).
> >   * I've had no time to check, but it should fix the drag'n'drop
> > bugs (e.g. moving tabs).

>
> As far as testing goes I confirm that this solves the drag'n'drop as
> well as the tooltip issue. Yeah!
>
> I didn't merge it because I'd rather someone else to have a look at
> the commits before.
>

I read the commit log and its diff. Everything seems me fine, but one
thing: the TOR_SOCKS_PORT set in /etc/environment by commit 6a2de87. It
seems me dangerous to set the socks port meant for the web browser only
(for stream isolation) as a global environment variable with such a
general name.

In addition to that, next commit (6629701) reads:

    Move some network.proxy.* prefs to user.js.


    It seems that they are not taken into account, with Torbutton
    1.5.x, unless set as user prefs.


And adds:

    +// Proxy and proxy security
    +user_pref("network.proxy.socks", "127.0.0.1");
    +user_pref("network.proxy.socks_port", 9063);
    +user_pref("network.proxy.socks_remote_dns", true);
    +user_pref("network.proxy.type", 1);


So I wonder if the previous commit setting environment is actually
useful. It it is, I would prefer to set these environment variables set
for iceweasel only, e.g. in the wrapper that we would probably create
anyway to solve https://tails.boum.org/todo/dont_autostart_iceweasel/.

Once there is an answer to this point I think we can merge this branch.

Cheers,