Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#5965 closed task (fixed)

Flag important sections of Torbutton code for preservation

Reported by: mikeperry Owned by: mikeperry
Priority: High Milestone:
Component: TorBrowserButton Version:
Severity: Keywords: MikePerry201206
Cc: n8fr8, gk Actual Points: 5
Parent ID: #5709 Points: 5
Reviewer: Sponsor:

Description

I should do a pass over the Torbutton code and mark the sections that we want to keep (or toss) somehow, to make #5709+#1506 easier.

Child Tickets

Change History (6)

comment:1 Changed 7 years ago by mikeperry

Actual Points: 4
Cc: n8fr8 gk added
Points: 4

Ok. This is done in mikeperry/bug5965. I've commented each function, object, and XPCOM component with "Bug 1506" and a Priority level. P5 is highest priority. Here's the level descriptions:

P0: Toggle mode code. It needs to die.
P1: Feature we could live without/do another way
P2: Mildly useful feature/enhancement
P3: Fingerprinting and linkability defenses; more useful features
P4: Important privacy features, including New Identity
P5: Proxy bypass/state leak issues

Some functions were given multiple priority levels (one for Android, one for Tor Browser, one for Firefox). The reasoning is described in the comments.

The idea is that these priority levels will guide #1506. We'd start with P5 code first and work down, probably stopping at P2 or P1.

Do either of you have any additional suggestions to help make your lives easier?

comment:2 Changed 7 years ago by mikeperry

comment:3 Changed 7 years ago by mikeperry

Here's the counts for each priority level:

P0: 58
P1: 25
P2: 15
P3: 12
P4: 17
P5: 3

comment:4 Changed 7 years ago by mikeperry

Actual Points: 45
Points: 45
Resolution: fixed
Status: newclosed

Merged in origin/master. I commented two more things in torbutton_update_status(), but I think Proxy Mobile already has them.

comment:5 in reply to:  1 ; Changed 7 years ago by gk

Replying to mikeperry:

Do either of you have any additional suggestions to help make your lives easier?

I might be a bit late to the party as you already closed the ticket, but yes I have some additional suggestions in random order:

1) A nit and maybe not worth fixing: "P0: Toogle mode code. It needs to die." -> "P0: It needs to die." as in jshooks.js is no toggle mode related code (and torbutton_check_version() and... neither).

2) Throw out FF3 related stuff as nobody is seriously supporting it anymore.

3) torbutton_test_settings() should die as well and get rather implemented as an own test in a test harness.

4) Remove preferences cruft (e.g. setting browser.safebrowsing.remoteLookups) and other cruft (contents.rdf).

5) What about torbutton_util.js and torcookie.js? Priority comments are missing in them.

comment:6 in reply to:  5 Changed 7 years ago by mikeperry

Replying to gk:

Replying to mikeperry:

Do either of you have any additional suggestions to help make your lives easier?

I might be a bit late to the party as you already closed the ticket, but yes I have some additional suggestions in random order:

1) A nit and maybe not worth fixing: "P0: Toogle mode code. It needs to die." -> "P0: It needs to die." as in jshooks.js is no toggle mode related code (and torbutton_check_version() and... neither).

2) Throw out FF3 related stuff as nobody is seriously supporting it anymore.

Yeah. I imagine this wisdom will be part of bringing up the Android dev up to speed on XPCOM in general, unless they're already an XPCOM wizard, in which case it should be obvious.

3) torbutton_test_settings() should die as well and get rather implemented as an own test in a test harness.

This is more for testing that your proxy settings are actually working for non-bundled users that manually installed everything.

4) Remove preferences cruft (e.g. setting browser.safebrowsing.remoteLookups) and other cruft (contents.rdf).

Yeah, the TBB prefs.js is our canonical list of pref changes. I've also now marked contents.rdf as P0.

5) What about torbutton_util.js and torcookie.js? Priority comments are missing in them.

Ok, added and pushed to origin/master.

Note: See TracTickets for help on using tickets.