Tails

#1 Updated by anonym 2015-09-16 13:07:18

• Status changed from Confirmed to In Progress

Applied in changeset commit:36016e8748588d2adce7b4ff92a09471f983b4ee.

#3 Updated by anonym 2015-09-16 13:07:48

• Assignee deleted (anonym)
• % Done changed from 0 to 50
• QA Check set to Ready for QA
• Feature Branch set to bugfix/9896-abstract-chroot-browser-configs

#4 Updated by intrigeri 2015-09-16 13:58:42

• Assignee set to anonym
• QA Check changed from Ready for QA to Info Needed

Great!

I wonder if this should be postponed to a major release. It’s more code polishing than a bugfix. Now, if we have good test coverage of the affected aspects, hey, why not sneak it in :)

#5 Updated by anonym 2015-09-16 14:59:15

• Assignee deleted (anonym)
• Target version changed from Tails_1.6 to Tails_1.7
• QA Check changed from Info Needed to Ready for QA

intrigeri wrote:
> Great!
>
> I wonder if this should be postponed to a major release. It’s more code polishing than a bugfix. Now, if we have good test coverage of the affected aspects, hey, why not sneak it in :)

Yeah, let’s postpone. The 1.7~rc1 would also potentially give some nice test coverage.

#6 Updated by intrigeri 2015-12-05 13:51:53

• Priority changed from Normal to Low
• Target version changed from Tails_1.7 to Tails_2.0

Postponing to next major release, and sneaking onto my plate with low-prio. Not a blocker for 2.0.

#7 Updated by intrigeri 2015-12-14 10:00:50

• Assignee set to intrigeri

#9 Updated by intrigeri 2015-12-17 11:07:32

• Target version changed from Tails_2.0 to Tails_2.2

Doesn’t qualify for a freeze exception IMO.

#10 Updated by intrigeri 2016-01-11 00:03:20

• Priority changed from Low to Normal

#11 Updated by intrigeri 2016-01-11 00:42:01

• Assignee changed from intrigeri to anonym
• % Done changed from 50 to 60
• QA Check changed from Ready for QA to Info Needed

I think I’d prefer this layout:

/usr/share/tails/chroot-browsers
  \
   common --> {prefs.js,userChrome.css}
   i2p    --> {prefs.js,userChrome.css} 
   unsafe --> {prefs.js,userChrome.css}

… to avoid dropping too much stuff at the root of /usr/share/tails/, and be generally clearer when inspecting this directory (it makes it immediately clear which browsers are chrooted). What do you think?

The comments in /usr/share/tails/chroot-browser/userChrome.css say why stuff won’t work in the I2P browser, but they don’t say why it’s OK to disable stuff in the Unsafe Browser. Not sure it’s a problem, your call.

Then I was looking for design doc that would need updating, but there was none so I added it on the devel branch with commit:6d33424. So this branch should add links to the shared config directory on both pages. I almost did it and then realized it would be wasting time if we rename the config dirs, so I’ll put it aside for the moment.

Other that than, code review passes, and it still builds fine!

#12 Updated by anonym 2016-02-04 17:45:40

• Assignee changed from anonym to intrigeri
• % Done changed from 60 to 70
• QA Check changed from Info Needed to Ready for QA

intrigeri wrote:
> I think I’d prefer this layout:
>
> […]
>
> … to avoid dropping too much stuff at the root of /usr/share/tails/, and be generally clearer when inspecting this directory (it makes it immediately clear which browsers are chrooted). What do you think?

Agreed, but I kept the -browser suffix given how the scripts implement this.

> The comments in /usr/share/tails/chroot-browser/userChrome.css say why stuff won’t work in the I2P browser, but they don’t say why it’s OK to disable stuff in the Unsafe Browser. Not sure it’s a problem, your call.

I added a few clarifications, but for the most part there’s a general argument why feature X is bad and should be discouraged, and that is enough imho.

> Then I was looking for design doc that would need updating, but there was none so I added it on the devel branch with commit:6d33424. So this branch should add links to the shared config directory on both pages. I almost did it and then realized it would be wasting time if we rename the config dirs, so I’ll put it aside for the moment.

Thanks! I always forget this. Now fixed.

#13 Updated by intrigeri 2016-02-05 17:19:22

• % Done changed from 70 to 80

Cool. Code review passes. I’ll run some tests before merging.

#14 Updated by intrigeri 2016-02-09 20:37:22

• blocked by Bug #11097: Most browser test suite scenarios fail on ISO images built from the devel branch added

#15 Updated by intrigeri 2016-02-09 20:39:00

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

Please reassign to me once Bug #11097 is addressed, or if you confirm you see the test suite pass on an ISO built from the current version of this branch.

#16 Updated by intrigeri 2016-02-11 12:26:14

• Assignee changed from anonym to intrigeri

#17 Updated by intrigeri 2016-02-11 12:26:27

• QA Check changed from Dev Needed to Ready for QA

#18 Updated by intrigeri 2016-02-11 14:05:29

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

Looks like the test suite needs a trivial update:

    And I configure the Unsafe Browser to check for updates more frequently                               # features/step_definitions/unsafe_browser.rb:168
      Could not append to '/usr/share/tails/unsafe-browser/prefs.js':

      /bin/sh: 1: cannot create /usr/share/tails/unsafe-browser/prefs.js: Directory nonexistent
      .
      <false> is not true. (Test::Unit::AssertionFailedError)
      ./features/support/helpers/vm_helper.rb:523:in `block in file_append'
      ./features/support/helpers/vm_helper.rb:521:in `each'
      ./features/support/helpers/vm_helper.rb:521:in `file_append'
      ./features/step_definitions/unsafe_browser.rb:170:in `/^I configure the Unsafe Browser to check for updates more frequently$/'
      features/unsafe_browser.feature:62:in `And I configure the Unsafe Browser to check for updates more frequently'

#19 Updated by intrigeri 2016-02-11 14:17:27

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

anonym promptly pushed commit:f8b7ba35fe3a844c1cdd7104246aec9f30e14929 that I reviewed (ACK) but didn’t test yet.

#20 Updated by intrigeri 2016-02-11 14:20:16

• Status changed from In Progress to Fix committed
• % Done changed from 80 to 100

Applied in changeset commit:525d469b5b286ba43844fb8d69f7dc4150e36cb1.

#22 Updated by intrigeri 2016-02-11 14:23:13

• Status changed from Fix committed to In Progress
• Assignee changed from intrigeri to anonym
• % Done changed from 100 to 90
• QA Check changed from Ready for QA to Dev Needed

#24 Updated by anonym 2016-02-11 15:26:12

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> Oops, I’ve merged too quickly. Now I see:
>
> […]
>
> I suggest you run features/unsafe_browser.feature:58 before re-submitting for QA :)

Sorry, I was too fast indeed. Fixed in commit:c4c08a0.

#25 Updated by anonym 2016-02-11 15:55:07

• Status changed from In Progress to Fix committed

So I actually merged it into stable, but please verify that you like commit:c4c08a0, and then properly close this ticket.

#26 Updated by intrigeri 2016-02-12 11:00:49

• % Done changed from 90 to 100

Applied in changeset commit:4b514f13b7e93dd2b6e464d296e4e62053dc634e.

#28 Updated by intrigeri 2016-02-12 13:18:29

• Target version changed from Tails_2.2 to Tails_2.0.1

#29 Updated by intrigeri 2016-02-13 11:45:24

• Assignee deleted (intrigeri)
• QA Check changed from Ready for QA to Pass

#30 Updated by intrigeri 2016-02-18 20:45:34

• Status changed from Fix committed to Resolved