Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-15 Thread intrigeri
anonym wrote (14 Oct 2014 11:57:02 GMT) :
 Done. I've successfully built an image which seem to have a working Tor
 Browser. Please merge into devel and testing again.

Done, will push and update Redmine at the same time as a bunch of
other branches, after testing a bit devel + these branches (I've
already tested all that in experimental, but well..)
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-15 Thread intrigeri
intrigeri wrote (15 Oct 2014 13:47:19 GMT) :
 Done, will push and update Redmine at the same time as a bunch of
 other branches, after testing a bit devel + these branches (I've
 already tested all that in experimental, but well..)

s/devel/testing/, of course.
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-14 Thread anonym
13/10/14 22:17, intrigeri wrote:
 Hi,
 
 anonym wrote (13 Oct 2014 15:54:06 GMT) :
 11/10/14 12:40, intrigeri wrote:
 1. install_debian_extensions() could take the list of packaged
extensions as arguments (after the tbb_install one); sure,
that's cosmetic.
 
 Done in commit a41768b.
 
 It's counter intuitive to me that the list of packages must be passed
 as a single argument. This construction is rarely seen elsewhere when
 passing lists of arguments. I think I would instead:
 
 # arguments: the destination directory, followed by a list of
 # extension packages
 install_debian_extensions() {
local destination
destination=${1}
shift
apt-get install --yes $@
[...]
 
 What do you think?

TBH, I considered it (I much prefer this convention) but felt rather the
opposite, e.g. I do not remember to see it used very widely. I'm
switching happily.

 2. BUNDLES should be moved to a configuration file, which first would
more clearly separate code from configuration, and second would
simplify the release process (after verifying sha256sums.txt, just
copy it to the right place).
 
 I separated all data (url also, since it can be expected to change
 frequently) from the hook in commit d46be96.
 
 Cool. I'm happy with the data separation, but I'm not happy with the
 implementation. 

Sorry, I completely understand you; in retrospect, the variable names
looks terrible. I admit I wasn't putting my full attention to these
commits, as I was working on several things in parallel. I've applied
all your suggestions.

 (BTW: For conflict resolution with the stuff from
 feature/tor-browser-4.0, have a look at how it was done for experimental
 in commit 1e9e1442e4b3ad8e82492ebd2d856ec45c02a884)
 
 I suspect I'll be able to merge feature/tor-browser-4.0 first. If this
 happens, then I'll let you merge the resulting testing branch into the
 refactoring one, and deal with the conflict resolution.

Just tell me when.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-14 Thread intrigeri
Hi,

anonym wrote (14 Oct 2014 09:29:26 GMT) :
 I've applied all your suggestions.

Code changes look good, thanks! Not tested yet.

 (BTW: For conflict resolution with the stuff from
 feature/tor-browser-4.0, have a look at how it was done for experimental
 in commit 1e9e1442e4b3ad8e82492ebd2d856ec45c02a884)
 
 I suspect I'll be able to merge feature/tor-browser-4.0 first. If this
 happens, then I'll let you merge the resulting testing branch into the
 refactoring one, and deal with the conflict resolution.

 Just tell me when.

Now! :)

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-14 Thread anonym
14/10/14 00:57, intrigeri wrote:
 anonym wrote (13 Oct 2014 15:54:06 GMT) :
 Please merge feature/8031-refactor-10-rbb-hook into devel and testing if
 everything looks good!
 
 Also:
 
 --- a/wiki/src/contribute/release_process/tor-browser.mdwn
 +++ b/wiki/src/contribute/release_process/tor-browser.mdwn
 @@ -4,20 +4,28 @@
  
  Have a look at
  
 -* https://archive.torproject.org/tor-package-archive/torbrowser/
 +* http://archive.torproject.org/tor-package-archive/torbrowser/
  * http://www.torproject.org/dist/torbrowser/
  * http://people.torproject.org/~mikeperry/builds/
 -* https://people.torproject.org/~linus/builds/
 +* http://people.torproject.org/~linus/builds/
 
 I'd rather see the RM use HTTPS, and then explicitly drop the s (if
 the limitation is still applicable) in the next step that, I suppose,
 explains why this change was made (Then update the url to the one
 chosen above).
 
 As a bonus, being explicit about it will avoid seeing the RM
 copy'n'paste from their browser URL bar a HTTPS URL that won't work at
 ISO build time, after being redirected e.g. thanks to HTTPS
 Everywhere :)

Makes sense. I found it easier to automate the release process, see
commit b36270b.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-14 Thread intrigeri
anonym wrote (14 Oct 2014 09:57:23 GMT) :
 Makes sense. I found it easier to automate the release process, see
 commit b36270b.

Great. Added two small fixes on top, pushing.

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-14 Thread anonym
13/10/14 22:18, intrigeri wrote:
 anonym wrote (13 Oct 2014 19:47:19 GMT) :
 See the note in `wiki/src/contribute/release_process/tor-browser.mdwn`
 of that branch. Basically, when using https://;, apt-cacher-ng will
 break, which e.g. Vagrant uses by default.
 
 I thought this was fixed by the version from wheezy-backports. If so,
 why aren't we using it?

Nope. I couldn't get the one in Wheezy to work at all.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-14 Thread intrigeri
anonym wrote (14 Oct 2014 11:54:38 GMT) :
 I thought this was fixed by the version from wheezy-backports. If so,
 why aren't we using it?

 Nope. I couldn't get the one in Wheezy to work at all.

I wrote wheezy-*backports*, so maybe there's a misunderstanding here?

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread anonym
11/10/14 13:04, intrigeri wrote:
 Hi,
 
 also, the current overall state of how we install and configure the
 browser at install time is currently spread over these hooks (at
 least):
 
  * 10-tbb
  * 12-remove_unwanted_browser_searchplugins
  * 13-override-tbb-branding
  * 14-add_localized_browser_searchplugins
  * 14-generate-tor-browser-profile
  * 15-symlink-places.sqlite
 
 I see numerous small problems with that, e.g.
 14-generate-tor-browser-profile's name is misleading, once you
 consider we now have a create_default_profile() function in 10-tbb.

True. The name should be `14-generate-tor-browser-profile-for-live-user`
or similar.

There's a comment inside it which is incorrect:

 # Generate Tor Browser profile at build time so it won't reside in RAM

When it's copied from /etc/skel into /home/amnesia, it will live in RAM.
What were we thinking here, really? To me it seems the only real use of
this is so that we can do the persistent bookmarks symlink in
`15-symlink-places.sqlite`.

 Also, the generate-tor-browser-profile script is only useful at ISO
 build time, so it probably shouldn't live in /usr/local/bin/.
 I'm pretty sure I would easily find more similar issues if I had
 a look at the other hooks.

Well, it's used in
`config/chroot_local-includes/usr/local/bin/tor-browser` too so that the
behaviour is at least more similar to how it was when we had Iceweasel.
Otherwise, removing ~/.tor-browser means that you cannot just run
`tor-browser` to get a fresh profile without the bookmarks symlink from
`15-symlink-places.sqlite`. You need to copy the profile from
/etc/tor-browser/profile manually.

Feature or regression, I don't know...

Perhaps 15-symlink-places.sqlite hook should be removed, and some
adapted persistent bookmarks symlink-creation code put into
`generate-tor-browser-profile` instead? Then there's no need for for
/etc/skel/.tor-browser, which may save a few KB from the iso size. The
bigger benefit is that things gets less confusing and more consistent.

 It would be good to take a step back and see if our current way of
 organizing things related to this topic is clear and robust enough.
 Maybe as a goal for 2.0, on the grounds that taking care of the
 general state of our codebase is definitely something we need to do to
 keep it maintainable on the long term?

Agreed. Filed as #8090.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread anonym
11/10/14 12:40, intrigeri wrote:
 Hi,
 
 for the record, I've already reviewed and merged the branches that fix
 #8057 and #8035.
 
 Now, reviewing the branch for #8031.
 
 0. The script looks much better and understandable to me, congrats!

\o/

 1. install_debian_extensions() could take the list of packaged
extensions as arguments (after the tbb_install one); sure,
that's cosmetic.

Done in commit a41768b.

 2. BUNDLES should be moved to a configuration file, which first would
more clearly separate code from configuration, and second would
simplify the release process (after verifying sha256sums.txt, just
copy it to the right place).

I separated all data (url also, since it can be expected to change
frequently) from the hook in commit d46be96.

 3. Please apply tails-greeter_fix-browser-localization-path.patch to
the Greeter's master branch, so that we simply can't forget to
migrate it there: as soon as a new Greeter with this patch is
uploaded, ISO builds will start failing so you'll automatically be
reminded of dropping the patch from our main Git repo.

Done in commit ca74f94.

 I'm going to merge the current state of this branch, since it's
 already a clear improvement over the current situation. Still, I'd
 rather not call this complete until #2 and #3 are addressed. #1 would
 be a nice bonus, and seems trivial, but not a blocker.

Please merge feature/8031-refactor-10-rbb-hook into devel and testing if
everything looks good!

(BTW: For conflict resolution with the stuff from
feature/tor-browser-4.0, have a look at how it was done for experimental
in commit 1e9e1442e4b3ad8e82492ebd2d856ec45c02a884)

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread anonym
13/10/14 20:14, sajol...@pimienta.org wrote:
 I haven't followed this thread at all, but while building Tails Jessie
 with that branch in it failed because of the redirection from
 http://people.torproject.org/ to https://people.torproject.org/.
 
 So why not using https:// instead of http:// in
 config/chroot_local-includes/usr/share/tails/tbb-dist-url.txt.

See the note in `wiki/src/contribute/release_process/tor-browser.mdwn`
of that branch. Basically, when using https://;, apt-cacher-ng will
break, which e.g. Vagrant uses by default.

 I'm not saying that the branch doesn't work but it really failed for me
 maybe because I still use some weird proxying based on squid-deb-proxy...

Perhaps. Please file a bug with all details.

Next, why not test Vagrant now? :)

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread intrigeri
Hi,

anonym wrote (13 Oct 2014 15:54:06 GMT) :
 11/10/14 12:40, intrigeri wrote:
 1. install_debian_extensions() could take the list of packaged
extensions as arguments (after the tbb_install one); sure,
that's cosmetic.

 Done in commit a41768b.

It's counter intuitive to me that the list of packages must be passed
as a single argument. This construction is rarely seen elsewhere when
passing lists of arguments. I think I would instead:

# arguments: the destination directory, followed by a list of
# extension packages
install_debian_extensions() {
   local destination
   destination=${1}
   shift
   apt-get install --yes $@
   [...]

What do you think?

 2. BUNDLES should be moved to a configuration file, which first would
more clearly separate code from configuration, and second would
simplify the release process (after verifying sha256sums.txt, just
copy it to the right place).

 I separated all data (url also, since it can be expected to change
 frequently) from the hook in commit d46be96.

Cool. I'm happy with the data separation, but I'm not happy with the
implementation. Notes follow:

 +install_debian_extensions ${DEBIAN_EXT} ${TBB_EXT}

This feels hard to understand while reading the code: $DEBIAN_EXT is
a list of Debian package names, and $TBB_EXT is a filesystem path.
I suggest renaming these variables to $DEBIAN_EXT_PKGS and
$TBB_EXT_DIR. IMO that's a blocker.

 +TBB_SHA256SUMS=/usr/share/tails/tbb-sha256sums.txt
 [...]
 +TBB_DIST_URL=/usr/share/tails/tbb-dist-url.txt

This feels very obscure too. The first time I read how one uses such
variables:

 +TBB_BUNDLES_BASE_URL=$(cat ${TBB_DIST_URL})/${VERSION}

... I thought What?! cat'ing a URL? = I think that a variable whose
value is the path to a file that itself contains some data should
instead be called e.g. TBB_DIST_URL_FILE, or whatever you think is
better, but certainly not TBB_DIST_URL (especially since it's not
consistent with how other very similar variables are called and used).
I think that's definitely a blocker.

Also, TBB_BUNDLES feels weird, as TBB means Tor Browser Bundle.
TBB_TARBALLS, or TBB_ARCHIVES, instead? (I don't see that one as
a blocker, but I suppose that while you're at it and testing things
after fixing the rest, you can as well take care of this one -- if
not, no big deal :)

 3. Please apply tails-greeter_fix-browser-localization-path.patch to
the Greeter's master branch, so that [...]

 Done in commit ca74f94.

Great!

 (BTW: For conflict resolution with the stuff from
 feature/tor-browser-4.0, have a look at how it was done for experimental
 in commit 1e9e1442e4b3ad8e82492ebd2d856ec45c02a884)

I suspect I'll be able to merge feature/tor-browser-4.0 first. If this
happens, then I'll let you merge the resulting testing branch into the
refactoring one, and deal with the conflict resolution.

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread intrigeri
anonym wrote (13 Oct 2014 19:47:19 GMT) :
 See the note in `wiki/src/contribute/release_process/tor-browser.mdwn`
 of that branch. Basically, when using https://;, apt-cacher-ng will
 break, which e.g. Vagrant uses by default.

I thought this was fixed by the version from wheezy-backports. If so,
why aren't we using it?
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread intrigeri
anonym wrote (13 Oct 2014 13:21:28 GMT) :
 # Generate Tor Browser profile at build time so it won't reside in RAM

 When it's copied from /etc/skel into /home/amnesia, it will live in RAM.
 What were we thinking here, really? To me it seems the only real use of
 this is so that we can do the persistent bookmarks symlink in
 `15-symlink-places.sqlite`.

That's my understanding too.

 Also, the generate-tor-browser-profile script is only useful at ISO
 build time, so it probably shouldn't live in /usr/local/bin/.
 I'm pretty sure I would easily find more similar issues if I had
 a look at the other hooks.

 Well, it's used in
 `config/chroot_local-includes/usr/local/bin/tor-browser` too so that the
 behaviour is at least more similar to how it was when we had Iceweasel.
 Otherwise, removing ~/.tor-browser means that you cannot just run
 `tor-browser` to get a fresh profile without the bookmarks symlink from
 `15-symlink-places.sqlite`. You need to copy the profile from
 /etc/tor-browser/profile manually.

 Feature or regression, I don't know...

Thanks for the explanation!

To put it mildly, I'm not enthusiastic at the idea of maintaining code
that supports this usecase, at all. If someone removes critical files
and want to see them back, they can as well reboot. I would just drop
the part about profile generation from start_browser() in
/usr/local/bin/tor-browser, so that generate-tor-browser-profile
doesn't have to support forever this obscure usecase (of the script)
that will inevitably be broken someday.

 Perhaps 15-symlink-places.sqlite hook should be removed, and some
 adapted persistent bookmarks symlink-creation code put into
 `generate-tor-browser-profile` instead? Then there's no need for for
 /etc/skel/.tor-browser, which may save a few KB from the iso size. The
 bigger benefit is that things gets less confusing and more consistent.

I'm not sure I'm following, but anything that makes all this stuff
less confusing will also make me happier :)

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-13 Thread intrigeri
anonym wrote (13 Oct 2014 15:54:06 GMT) :
 Please merge feature/8031-refactor-10-rbb-hook into devel and testing if
 everything looks good!

Also:

--- a/wiki/src/contribute/release_process/tor-browser.mdwn
+++ b/wiki/src/contribute/release_process/tor-browser.mdwn
@@ -4,20 +4,28 @@
 
 Have a look at
 
-* https://archive.torproject.org/tor-package-archive/torbrowser/
+* http://archive.torproject.org/tor-package-archive/torbrowser/
 * http://www.torproject.org/dist/torbrowser/
 * http://people.torproject.org/~mikeperry/builds/
-* https://people.torproject.org/~linus/builds/
+* http://people.torproject.org/~linus/builds/

I'd rather see the RM use HTTPS, and then explicitly drop the s (if
the limitation is still applicable) in the next step that, I suppose,
explains why this change was made (Then update the url to the one
chosen above).

As a bonus, being explicit about it will avoid seeing the RM
copy'n'paste from their browser URL bar a HTTPS URL that won't work at
ISO build time, after being redirected e.g. thanks to HTTPS
Everywhere :)

Cheers!
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-11 Thread intrigeri
Hi,

also, the current overall state of how we install and configure the
browser at install time is currently spread over these hooks (at
least):

 * 10-tbb
 * 12-remove_unwanted_browser_searchplugins
 * 13-override-tbb-branding
 * 14-add_localized_browser_searchplugins
 * 14-generate-tor-browser-profile
 * 15-symlink-places.sqlite

I see numerous small problems with that, e.g.
14-generate-tor-browser-profile's name is misleading, once you
consider we now have a create_default_profile() function in 10-tbb.
Also, the generate-tor-browser-profile script is only useful at ISO
build time, so it probably shouldn't live in /usr/local/bin/.
I'm pretty sure I would easily find more similar issues if I had
a look at the other hooks.

It would be good to take a step back and see if our current way of
organizing things related to this topic is clear and robust enough.
Maybe as a goal for 2.0, on the grounds that taking care of the
general state of our codebase is definitely something we need to do to
keep it maintainable on the long term?

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-11 Thread sajolida
intrigeri wrote:
 +[Tor Browser](https://www.torproject.org/projects/torbrowser.html.en) is a 
 web
 +browser based on [Mozilla Firefox](http://getfirefox.com) and configured to
 +protect your anonymity.
 
 Tor Browser is not only *configured*, it is also *modified*. I think
 we should make it clear here, otherwise the reader may believe that
 proper configuration of Firefox could possibly be enough to achieve
 the same level of protection that Tor Browser provides.

Done in 844b296.

 -Once started, the so called router console will open in Tor Browser,
 +Once started, the so called router console will open in span 
 class=applicationTor Browser/span,
 
 This conflicts with the current state of the testing branch (where the
 branch that migrates to a dedicated I2P Browser has been merged).
 Please merge current testing, resolve conflicts, and resubmit. Thanks!

Done in 4feebff.

Resubmitting then...

-- 
sajolida

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-11 Thread intrigeri
sajol...@pimienta.org wrote (11 Oct 2014 11:19:35 GMT) :
 Resubmitting then...

Merged, thanks! (I thought the conflict would have been harder to
resolve, otherwise I would have done both changes myself, actually.
Sorry for the extra paperwork.)

Cheers,
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-09 Thread sajolida
intrigeri wrote:
 intrigeri wrote (03 Oct 2014 09:22:23 GMT) :
 I've seen you added automated tests, congrats. A few comments:
 
 FTR: all these concerns were addressed, some others were postponed via
 new tickets, and I've merged the branch.

I pushed my updates on the user documentation. Can you have a look at
524a8ea..1c1e912 and merge again?

-- 
sajolida

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-09 Thread intrigeri
sajol...@pimienta.org wrote (09 Oct 2014 15:07:08 GMT) :
 I pushed my updates on the user documentation. Can you have a look at
 524a8ea..1c1e912 and merge again?

Thanks, I'm on it.
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-08 Thread intrigeri
intrigeri wrote (03 Oct 2014 09:22:23 GMT) :
 I've seen you added automated tests, congrats. A few comments:

FTR: all these concerns were addressed, some others were postponed via
new tickets, and I've merged the branch.

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-02 Thread intrigeri
Hi,

great to see some initial work on the migration to FF31 in that
branch! I'm going to build a test ISO right away.

Wrt. commit 4ba3198 (Run Tor Browser using the TBB's shared
libraries), two questions:

  * Does this mean that without this commit, the Tor Browser was only
using system libraries (e.g. NSS)? If it is so, I think the design
doc needs to make it clear that we *really* need the Tor Browser
binary to use the bundled libs, and how it's implemented. Also,
I think we should have at least a manual test about this.

  * Doesn't the tor-launcher script need to do the same? (And then,
the I2P browser one too, and then maybe we should refactor this
somehow, if it needs to be replicated in 4 different places.)

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-10-02 Thread anonym
02/10/14 11:23, intrigeri wrote:
 Hi,
 
 great to see some initial work on the migration to FF31 in that
 branch! I'm going to build a test ISO right away.
 
 Wrt. commit 4ba3198 (Run Tor Browser using the TBB's shared
 libraries), two questions:
 
   * Does this mean that without this commit, the Tor Browser was only
 using system libraries (e.g. NSS)?

For some reason they *were* loaded pre-31esr even without setting
LD_LIBRARY_PATH.

 If it is so, I think the design
 doc needs to make it clear that we *really* need the Tor Browser
 binary to use the bundled libs, and how it's implemented.

Sure, done.

 Also,
 I think we should have at least a manual test about this.

Done in the branch.

   * Doesn't the tor-launcher script need to do the same?

Oops, yes of course. Now fixed.

 (And then,
 the I2P browser one too, and then maybe we should refactor this
 somehow, if it needs to be replicated in 4 different places.)

Good idea, now done.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-30 Thread intrigeri
Hi,

FYI I've set up a Jenkins job so that feature/tor-browser-bundle is
built daily, and on Git push. The resulting ISO images live in:

  http://nightly.tails.boum.org/build_Tails_ISO_feature-tor-browser-bundle/

anonym, how about sending a call for testing on -testers@?

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-30 Thread anonym
30/09/14 19:58, intrigeri wrote:
 Hi,
 
 FYI I've set up a Jenkins job so that feature/tor-browser-bundle is
 built daily, and on Git push. The resulting ISO images live in:
 
   http://nightly.tails.boum.org/build_Tails_ISO_feature-tor-browser-bundle/

Thanks!

 anonym, how about sending a call for testing on -testers@?

Good idea! Done!

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-27 Thread intrigeri
anonym wrote (26 Sep 2014 17:41:07 GMT) :
 What do you consider to be left to do, as musts, before this is
 merge-ready?

To answer this question, I need to have a clear view of what should be
improved, that is, a parent ticket and a set of subtasks.

Cheers!
-- 
intrigeri
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-26 Thread anonym
Hi,

The last bug I've discovered has now been fixed (#7944). A test for
persistent bookmarks has been added (and it passes) as discussed privately.

What do you consider to be left to do, as musts, before this is
merge-ready? FYI I promise a strong post-freeze commitment to fix any
bugs or minor issues (e.g. those discussed in your previous email that
haven't been resolved yet) remaining after a merge.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-24 Thread anonym
Hi,

First of all, not that I've fixed a few bugs and updated the automated
test suite so it deals with the TBB migration.

21/09/14 00:51, intrigeri wrote:
 Hi,
 
 anonym wrote (20 Sep 2014 01:20:00 GMT) :
 +Explanation: Block installation of iceweasel until it has been removed 
 from our APT repo
 +Package: iceweasel
 +Pin: origin deb.tails.boum.org
 +Pin-Priority: -1

 I'm curious why we need that at all, and the explanation isn't very
 convincing: even once we remove it from our own APT repo, it will be
 available in the Debian ones, so I don't get it.
[...]
 Something like:
 
   Explanation: keep our fake equivs-generated iceweasel package
   Package: iceweasel
   Pin: origin 
   Pin-Priority: 1020
 
 might be just enough to express exactly what we want to say
 (untested), in a way that would work in more situations.

I tested it, and it didn't work:

[...]
The following NEW packages will be installed:
  libmozjs24d xulrunner-24.0
The following packages will be DOWNGRADED:
  iceweasel
0 upgraded, 2 newly installed, 1 downgraded, 0 to remove and 0 not
upgraded.
Need to get 21.8 MB of archives.
After this operation, 56.0 MB of additional disk space will be used.
E: There are problems and -y was used without --force-yes
P: Begin unmounting filesystems...
[...]

 Regarding config/chroot_local-hooks/10-tbb, a lot of the code could
 enjoy some refactoring. Currently, configuration, low-level processing
 and the high-level flow are too strongly intermingled for my taste.
 
 Could you please elaborate?
 
 I would suggest *naming* operations that are being done, [...]
 But oh well, it feels strange to pretend I can teach you anything wrt.
 software design and refactoring, you already know all this :)

I misunderstood the scope of what you meant. I have started some work in
this direction and will push it later just so that work doesn't block.

 +TBB_EXT=${TBB_INSTALL}/extensions

 I'm curious why we need to put extensions in a custom place, instead
 of letting them live in the place as in the TBB.
 
 I.e. directly in the browser profile skeleton at /etc/icewease/profile?
 Well, I'd like to just be able to copy the profile skeleton when
 creating a new profile without wasting space (well, RAM because tmpfs)
 on duplicating every extension.
 
 OK, makes a lot of sense. Make it clear in a comment? (I think we
 should aim at the smallest possible delta with the TB here, so
 documenting *why* this and that bit of our delta is needed will help
 whenever we try to make the delta smaller in the future, and someone
 will be asking exactly this kind of questions :)

Ok, I think I've improved it a bit now.

 +# We don't want tor-launcher to be part of the browser, and we need our
 +# patched stand-alone version any way.

 s/and/as/ ? Also, I suggest pointing to the parent ticket that tracks
 upstreaming our changes.
 
 Well, I meant it as two separate reasons for doing that. I now realize
 the stand-alone in the second part creates some overlap with the first
 part, which may cause confusion. Would removing stand-alone make it
 clearer?
 
 Ah, I got it. How about: We don't want tor-launcher to be part of the
 regular browser profile. Moreover, for the stand-alone tor-launcher we
 use, we need our patched version. So, the version shipped in the TB
 really is not useful for us.

Looks better, indeed. Applied.

 In config/chroot_local-hooks/12-remove_unwanted_browser_searchplugins:

 +PLUGIN_DIR=/usr/local/lib/tor-browser/Browser/browser/searchplugins

 It seems that we're hard-coding the same path information in different
 places. How about setting TBB_INSTALL and friends in a common place,
 that can be sourced by all scripts that need it?
 
 Sure. I couldn't come up with a place where we already do this. Do you
 have any suggestion for a good location? /etc/live/config.d?
 
 I'm unsure whether we really want to export all these variables as
 part of the global system-wide environment. I would instead store them
 somewhere that can be sourced when needed.
 
 We already do similar things in auto/config (saving stuff to
 /etc/amnesia/) and auto/build (saving stuff to
 /usr/share/amnesia/build/). I think that /usr makes more sense than
 /etc, as what we want to save here is really static information about
 how/where vendor-provided software is setup in the ISO, rather than
 configuration = /usr/local/lib/tails-shell-library/tor-browser.sh,
 maybe? It might be that we need more than variables in there at some
 point, so bootstrapping a mini-shell-library with these doesn't seem
 too crazy.

Agreed. This is now done.

 -daddr 127.0.0.1 proto tcp syn mod multiport 
 destination-ports (9050 9061 9062 9151) {
 +daddr 127.0.0.1 proto tcp syn mod multiport 
 destination-ports (9050 9061 9062 9150) {

 It would be nice for anyone who has custom configuration that depends
 on the SOCKS port to keep the 9151 we've been using until now. OTOH,
 maybe people 

Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-20 Thread intrigeri
Hi,

anonym wrote (20 Sep 2014 01:20:00 GMT) :
 +Explanation: Block installation of iceweasel until it has been removed 
 from our APT repo
 +Package: iceweasel
 +Pin: origin deb.tails.boum.org
 +Pin-Priority: -1
 
 I'm curious why we need that at all, and the explanation isn't very
 convincing: even once we remove it from our own APT repo, it will be
 available in the Debian ones, so I don't get it.

 I'm not exactly sure why this is needed but without it our iceweasel
 package is pulled when an `apt-get dist-upgrade` is issued (which e.g.
 live-build does some time after the hooks). However, Debian's package
 isn't pulled. I suppose the Pin-Priority of the fake Iceweasel package
 installed (with `dpkg -i`) is lower than deb.t.b.o's 1005, but higher
 than the 990 we assign to the Debian repos.

Ah, right, got it. Actually, the installed (fake) package has priority
100, but it has a version higher than the one in Debian, and APT won't
downgrade unless the priority of an available version exceeds 1000.

I was confused by the comment: it's not about blocking *installation*
of the iceweasel package (since we do want our fake one to be
installed), it's about blocking upgrade from our fake package to the
version we have in our repo. Given this, and what we actually want to
express is don't upgrade the installed iceweasel package from an APT
repo, ever, I suspect a better, more generic Pin value could be
set: the current thing relies on the fact that the priority of all
repos that carry iceweasel is lower than 1000, except the ones we
explicitly tell about here. This seems to express in a quite
convoluted way what we want to say, and is also slightly fragile.

Something like:

  Explanation: keep our fake equivs-generated iceweasel package
  Package: iceweasel
  Pin: origin 
  Pin-Priority: 1020

might be just enough to express exactly what we want to say
(untested), in a way that would work in more situations.

 Regarding config/chroot_local-hooks/10-tbb, a lot of the code could
 enjoy some refactoring. Currently, configuration, low-level processing
 and the high-level flow are too strongly intermingled for my taste.

 Could you please elaborate?

I would suggest *naming* operations that are being done, using
functions. Currently, one really has to read all comments to have
a broad idea of what the code is doing. While doing this, one would
also clarify what's the input and output of each operation, instead of
relying that much on incrementally built global state.

E.g. I would create a download_and_verify_files() function, taking
a base URL, a destination directory, and a list of (filename, expected
hash) -- or drop the base URL and pass the full URL for each filename.
I would create a create_and_install_fake_iceweasel_pkg() function,
that would take a version number as its single parameter, and would
use its own tmpdir instead of global state, etc. At least naming the
major steps of the process would help, even if we're still using a lot
of global state :)

But oh well, it feels strange to pretend I can teach you anything wrt.
software design and refactoring, you already know all this :)

 +TBB_EXT=${TBB_INSTALL}/extensions
 
 I'm curious why we need to put extensions in a custom place, instead
 of letting them live in the place as in the TBB.

 I.e. directly in the browser profile skeleton at /etc/icewease/profile?
 Well, I'd like to just be able to copy the profile skeleton when
 creating a new profile without wasting space (well, RAM because tmpfs)
 on duplicating every extension.

OK, makes a lot of sense. Make it clear in a comment? (I think we
should aim at the smallest possible delta with the TB here, so
documenting *why* this and that bit of our delta is needed will help
whenever we try to make the delta smaller in the future, and someone
will be asking exactly this kind of questions :)

 The latter would break the read-while-loop later on since the first
 iteration would read an empty line. And beginning listing the first of
 the sha256/tarball lines immediately after the quotation breaks the flow.

Fair enough.

[... snipping a lot of Fixed! replies from anonym -- thanks :) ...]

 +# We don't want tor-launcher to be part of the browser, and we need our
 +# patched stand-alone version any way.
 
 s/and/as/ ? Also, I suggest pointing to the parent ticket that tracks
 upstreaming our changes.

 Well, I meant it as two separate reasons for doing that. I now realize
 the stand-alone in the second part creates some overlap with the first
 part, which may cause confusion. Would removing stand-alone make it
 clearer?

Ah, I got it. How about: We don't want tor-launcher to be part of the
regular browser profile. Moreover, for the stand-alone tor-launcher we
use, we need our patched version. So, the version shipped in the TB
really is not useful for us.

 I'm not sure simple grep:ing (or similar) is enough since the em:id
 tag isn't unique (adblock-plus has several, for instance). Well, perhaps
 the 

Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-19 Thread Kill Your TV
On Thu, 18 Sep 2014 23:08:44 + (UTC)
intrigeri intrig...@boum.org wrote:


 +ln -s /usr/share/xul-ext/adblock-plus/ 
 ${TBB_EXT}/'{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}'
 +ln -s /usr/share/xul-ext/foxyproxy-standard/ 
 ${TBB_EXT}/foxypr...@eric.h.jung
 +ln -s /usr/share/xul-ext/torbutton/ ${TBB_EXT}/torbut...@torproject.org  

 It would be nice to take 15 minutes to try and extract the right-hand
 values dynamically. If it's not done after 15 minutes, give up :)

Perhaps something like this would work?


get_id(){
sed -nr 's/^\s+em:id(.+)\/em:id/\1/p' ${1}/install.rdf | head -n1
}

ADBLOCK_EXT=/usr/share/xul-ext/adblock-plus
FOXYPROXY_EXT=/usr/share/xul-ext/foxyproxy-standard/
TORBUTTON_EXT=/usr/share/xul-ext/torbutton/
ADBLOCK_UUID=$(get_id ${ADBLOCK_EXT})
FOXYPROXY_UUID=$(get_id ${FOXYPROXY_EXT})
TORBUTTON_UUID=$(get_id ${TORBUTTON_EXT})


-- 
GPG ID: 0x5BF72F42D0952C5A
Fingerprint: BD12 65FD 4954 C40A EBCB  F5D7 5BF7 2F42 D095 2C5A


signature.asc
Description: PGP signature
___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.

Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-19 Thread anonym
19/09/14 01:08, intrigeri wrote:
 Hi,
 
 I had a first look at anonym's work on the feature/tor-browser-bundle
 branch.

I don't want to block your and kytv's work, so I think I'll just leave
the behemoth commit as-is, but with a more informative commit message.
Force pushed. Unless you have an issue with this, you may start basing
work on this branch now.

I'll fix the automated tests on Monday. Oh, and if you've noticed that
our bookmarks doesn't work, I'm already aware of it. I had the intention
to create tickets about what's remaining, but time flied, and I'm afraid
that too will have to wait until Monday.

 Here are a few initial comments.
 
 +Explanation: Block installation of iceweasel until it has been removed from 
 our APT repo
 +Package: iceweasel
 +Pin: origin deb.tails.boum.org
 +Pin-Priority: -1
 
 I'm curious why we need that at all, and the explanation isn't very
 convincing: even once we remove it from our own APT repo, it will be
 available in the Debian ones, so I don't get it.

I'm not exactly sure why this is needed but without it our iceweasel
package is pulled when an `apt-get dist-upgrade` is issued (which e.g.
live-build does some time after the hooks). However, Debian's package
isn't pulled. I suppose the Pin-Priority of the fake Iceweasel package
installed (with `dpkg -i`) is lower than deb.t.b.o's 1005, but higher
than the 990 we assign to the Debian repos.

 Regarding config/chroot_local-hooks/10-tbb, a lot of the code could
 enjoy some refactoring. Currently, configuration, low-level processing
 and the high-level flow are too strongly intermingled for my taste.

Could you please elaborate?

 +TBB_EXT=${TBB_INSTALL}/extensions
 
 I'm curious why we need to put extensions in a custom place, instead
 of letting them live in the place as in the TBB.

I.e. directly in the browser profile skeleton at /etc/icewease/profile?
Well, I'd like to just be able to copy the profile skeleton when
creating a new profile without wasting space (well, RAM because tmpfs)
on duplicating every extension.

 +BUNDLES=$(cat EOF
 [...]
 +EOF
 +)
 
 I'm not sure why this wouldn't work:
 
   BUNDLES=
   [...]
   

The latter would break the read-while-loop later on since the first
iteration would read an empty line. And beginning listing the first of
the sha256/tarball lines immediately after the quotation breaks the flow.

 +mkdir -p ${TBB_INSTALL} ${TBB_PROFILE} ${TBB_EXT}
 
 Here, we don't implicitly rely on the fact $TBB_EXT is a subdir of
 $TBB_INSTALL, while the chown/chmod at the end of the script does.
 Please pick one (probably the slightly safer explicit version) to
 avoid potentially hard-to-debug future problems :)

Fixed!

 +APT_PROXY=$(sed -n 's/Acquire::http::Proxy \(.*\);/\1/p' 
 /etc/apt/apt.conf.d/00http-proxy)
 
 Instead, I would use something like:
 
   apt-config --format '%v' dump Acquire::http::Proxy

I wasn't aware of this. Now fixed.

 +curl -O ${TBB_DIST_URL}/${tarball}
 
 Please use long option names to ease reviewing, auditing and
 future maintenance.

Fixed!

 +# We'll use the en-US bundle as our basis ...
 
 Spurious space before 

Thanks!

 +TBB_PREP=${TMP}/tor-browser_en-US
 
 After extracting the bundle, I would explicitly check that this
 directory exists, to make things clearer if the top-level directory
 shipped in the tarball is renamed some day.

I opted for explicitly extracting the expected folder. If there's a name
change, tar will now fail with an informative error message.

 +tar -xJf ${TMP}/${MAIN_BUNDLE} -C ${TMP}
 and
 +tar xJf ${tarball} ${xpi}
 
 Two remarks here:
 
  * Better not use J, as any not-too-old tar auto-detects compression
anyway, and thus there's no need to encode the current compression
algorithm in our code.
  * Please pick one between -xf and xf, and use it consistently.

Thanks!

 +# We don't want tor-launcher to be part of the browser, and we need our
 +# patched stand-alone version any way.
 
 s/and/as/ ? Also, I suggest pointing to the parent ticket that tracks
 upstreaming our changes.

Well, I meant it as two separate reasons for doing that. I now realize
the stand-alone in the second part creates some overlap with the first
part, which may cause confusion. Would removing stand-alone make it
clearer?

 +# Remove TBB's torbutton since the Tor test will fail and about:tor
 +# will report an error. We'll install our own Torbutton later, which
 +# has the extensions.torbutton.test_enabled boolean pref as a workaround.
 
 Same here, please point to the relevant ticket.
 
 I would do all the iceweasel equivs things in a tempdir: if some day
 equivs creates other files than the one we're manually cleaning up,
 then the current code will happily put these files into /root in
 the SquashFS.

Thanks!

 +ln -s /usr/share/xul-ext/adblock-plus/ 
 ${TBB_EXT}/'{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}'
 +ln -s /usr/share/xul-ext/foxyproxy-standard/ 
 ${TBB_EXT}/foxypr...@eric.h.jung
 +ln -s /usr/share/xul-ext/torbutton/ 

Re: [Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-19 Thread anonym
20/09/14 00:02, Kill Your TV wrote:
 On Thu, 18 Sep 2014 23:08:44 + (UTC)
 intrigeri intrig...@boum.org wrote:
 
 
 +ln -s /usr/share/xul-ext/adblock-plus/ 
 ${TBB_EXT}/'{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}'
 +ln -s /usr/share/xul-ext/foxyproxy-standard/ 
 ${TBB_EXT}/foxypr...@eric.h.jung
 +ln -s /usr/share/xul-ext/torbutton/ ${TBB_EXT}/torbut...@torproject.org  
 
 It would be nice to take 15 minutes to try and extract the right-hand
 values dynamically. If it's not done after 15 minutes, give up :)
 
 Perhaps something like this would work?
 
 
 get_id(){
 sed -nr 's/^\s+em:id(.+)\/em:id/\1/p' ${1}/install.rdf | head -n1
 }

Unfortunately this won't work, see e.g. AdBlock Plus' install.rdf, which
has several em:id tags. To fix the indentation level might be
enough, but I don't know how the format is specified exactly.

Cheers!

___
Tails-dev mailing list
Tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
To unsubscribe from this list, send an empty email to 
tails-dev-unsubscr...@boum.org.


[Tails-dev] Migrating to (something closer to) the regular Tor Browser

2014-09-18 Thread intrigeri
Hi,

I had a first look at anonym's work on the feature/tor-browser-bundle
branch. Here are a few initial comments.

 +Explanation: Block installation of iceweasel until it has been removed from 
 our APT repo
 +Package: iceweasel
 +Pin: origin deb.tails.boum.org
 +Pin-Priority: -1

I'm curious why we need that at all, and the explanation isn't very
convincing: even once we remove it from our own APT repo, it will be
available in the Debian ones, so I don't get it.

Regarding config/chroot_local-hooks/10-tbb, a lot of the code could
enjoy some refactoring. Currently, configuration, low-level processing
and the high-level flow are too strongly intermingled for my taste.

 +TBB_EXT=${TBB_INSTALL}/extensions

I'm curious why we need to put extensions in a custom place, instead
of letting them live in the place as in the TBB.

 +BUNDLES=$(cat EOF
 [...]
 +EOF
 +)

I'm not sure why this wouldn't work:

  BUNDLES=
  [...]
  

 +mkdir -p ${TBB_INSTALL} ${TBB_PROFILE} ${TBB_EXT}

Here, we don't implicitly rely on the fact $TBB_EXT is a subdir of
$TBB_INSTALL, while the chown/chmod at the end of the script does.
Please pick one (probably the slightly safer explicit version) to
avoid potentially hard-to-debug future problems :)

 +APT_PROXY=$(sed -n 's/Acquire::http::Proxy \(.*\);/\1/p' 
 /etc/apt/apt.conf.d/00http-proxy)

Instead, I would use something like:

  apt-config --format '%v' dump Acquire::http::Proxy

 +curl -O ${TBB_DIST_URL}/${tarball}

Please use long option names to ease reviewing, auditing and
future maintenance.

 +# We'll use the en-US bundle as our basis ...

Spurious space before 

 +TBB_PREP=${TMP}/tor-browser_en-US

After extracting the bundle, I would explicitly check that this
directory exists, to make things clearer if the top-level directory
shipped in the tarball is renamed some day.

 +tar -xJf ${TMP}/${MAIN_BUNDLE} -C ${TMP}
and
 +tar xJf ${tarball} ${xpi}

Two remarks here:

 * Better not use J, as any not-too-old tar auto-detects compression
   anyway, and thus there's no need to encode the current compression
   algorithm in our code.
 * Please pick one between -xf and xf, and use it consistently.

 +# We don't want tor-launcher to be part of the browser, and we need our
 +# patched stand-alone version any way.

s/and/as/ ? Also, I suggest pointing to the parent ticket that tracks
upstreaming our changes.

 +# Remove TBB's torbutton since the Tor test will fail and about:tor
 +# will report an error. We'll install our own Torbutton later, which
 +# has the extensions.torbutton.test_enabled boolean pref as a workaround.

Same here, please point to the relevant ticket.

I would do all the iceweasel equivs things in a tempdir: if some day
equivs creates other files than the one we're manually cleaning up,
then the current code will happily put these files into /root in
the SquashFS.

 +ln -s /usr/share/xul-ext/adblock-plus/ 
 ${TBB_EXT}/'{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}'
 +ln -s /usr/share/xul-ext/foxyproxy-standard/ 
 ${TBB_EXT}/foxypr...@eric.h.jung
 +ln -s /usr/share/xul-ext/torbutton/ ${TBB_EXT}/torbut...@torproject.org

It would be nice to take 15 minutes to try and extract the right-hand
values dynamically. If it's not done after 15 minutes, give up :)

 +cp -r ${TBB_PREP}/Data/Browser/profile.default/* ${TBB_PROFILE}/

This doesn't copy hidden files, right? I would use rsync instead,
and try to preserve more inode information.

 +rm -rf ${TMP}

If `rm -r' works, better avoid the -f that will hide errors from `set -e'.

In config/chroot_local-hooks/12-remove_unwanted_browser_searchplugins:

 +PLUGIN_DIR=/usr/local/lib/tor-browser/Browser/browser/searchplugins

It seems that we're hard-coding the same path information in different
places. How about setting TBB_INSTALL and friends in a common place,
that can be sourced by all scripts that need it?

 +   sed -i '/^user_pref(extensions.torlauncher.prompt_at_startup/d' 
 ${pref}

I think that the dots might need to be escaped (and if there's
a problem here, yes, it was already in the previous code), but my sed
regexp foo is pretty low (I usually switch to Perl as soon as I'm
unsure).

 -daddr 127.0.0.1 proto tcp syn mod multiport 
 destination-ports (9050 9061 9062 9151) {
 +daddr 127.0.0.1 proto tcp syn mod multiport 
 destination-ports (9050 9061 9062 9150) {

It would be nice for anyone who has custom configuration that depends
on the SOCKS port to keep the 9151 we've been using until now. OTOH,
maybe people are installing scripts from the Internet that rely on the
default TBB port, so I'm unsure what's best. Is there a good reason to
change it?

 +cp -r /etc/tor-browser/profile ${USER_PROFILE}/profile.default

I would use rsync and preserve more inode information.

 +# Use the Tor Browser's hardcoded relative-patch only default profile

s/patch/patch/ ?

 +mkdir -p /usr/Data/Browser/profile.default/preferences

/usr/Data/, really? Is that the hardcoded