Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#18238 closed defect (fixed)

remove unused code and strings from Torbutton

Reported by: mcs Owned by: brade
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-torbutton, TorBrowserTeam201605R
Cc: mcs, gk, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

While waiting for Tor Browser builds to finish, Kathy started to look at what code and strings could be removed from Torbutton. This will be done with a series of patches to make review easier.

Child Tickets

Change History (11)

comment:1 Changed 4 years ago by cypherpunks

See #18233 for a suggestion on things to remove.

comment:2 Changed 4 years ago by mcs

For now, our focus is on removing dead code and strings that are not used by any existing features of Torbutton. Here is the first patch:
https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18238-A-01&id=39402307e28599be8658762047c9a59cdc3d91c0

comment:4 Changed 4 years ago by mcs

Keywords: TorBrowserTeam201602 added
Status: newneeds_information

Kathy and I noticed that the toolbarbutton (defined in src/chrome/content/torbutton.xul) has a hard-coded label of "Torbutton". Should it stay that way or should the button label be localized?

There is an unused entity named torbutton.button.label that is not otherwise used; it seems likely that it was intended for this purpose. But maybe this is a branding issue and we should keep the hard-coded Torbutton label and just remove the unused entity.

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

Status: needs_informationassigned

Replying to mcs:

But maybe this is a branding issue and we should keep the hard-coded Torbutton label and just remove the unused entity.

Yes, this sounds like a good idea.

comment:6 in reply to:  5 Changed 4 years ago by brade

Replying to gk:

Replying to mcs:

But maybe this is a branding issue and we should keep the hard-coded Torbutton label and just remove the unused entity.

Yes, this sounds like a good idea.

I am OK with this decision; I will remove the unused entity.
I do want to point out that it could be an issue if someone someone in China (or other locales) has an icon on their toolbar that has English text instead of their locale: it would stand out and a passerby could easily notice. However, the default is that toolbar icons do not have a text label so this seems like a less likely scenario.

comment:7 Changed 4 years ago by gk

Keywords: TorBrowserTeam201603 added; TorBrowserTeam201602 removed

comment:8 Changed 4 years ago by mcs

Keywords: TorBrowserTeam201604 added; TorBrowserTeam201603 removed

We left the month of March behind a couple of weeks ago.

comment:9 Changed 3 years ago by mcs

Keywords: TorBrowserTeam201605R added; TorBrowserTeam201604 removed
Status: assignedneeds_review

Since some of this work is done, we should consider merging it for the upcoming alpha release. There are three commits on brade's bug18238-01 branch that are ready for review, here:
https://gitweb.torproject.org/user/brade/torbutton.git/log/?h=bug18238-01
(please ignore the patches mentioned earlier in this ticket).

comment:10 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. This is commit 8eaca2e12c9c90c8af1cc85817956c2e1f554e52, e8f694c4fb513c92ec26603fdaa3be745d7648a5 and 58d67d50eb41caf0493126a859646a476d6700f4 on master.

comment:11 Changed 3 years ago by mcs

I filed #19256 for followup work.

Note: See TracTickets for help on using tickets.