[tbb-bugs] #28745 [Applications/Tor Browser]: THE Torbutton clean-up

[Tor Bug Tracker & Wiki]

#28745: THE Torbutton clean-up
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-torbutton, TorBrowserTeam201911  |  Actual Points:
Parent ID:  #30506                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by gk):

 * keywords:  tbb-torbutton, TorBrowserTeam201911R => tbb-torbutton,
     TorBrowserTeam201911
 * status:  needs_review => needs_revision


Comment:

 commit a12088fdc0d343d81b6ecf53faf06c91f8da27d5 looks good. I've applied
 it tor `torbutton`'s `master` (commit
 65e1e8ba67ba865e7b3ded74b7e4bbc5b86c721c)

 Some nits and a question for commit
 7e23b3cbf66717c1b9b3a107daf3147b54061a96:
 {{{
 +/* global ,
 }}}
 I think you don't want to have a `,` + whitespace there?

 Are you sure we want to do
 {{{
 -  let firstPartyDomain = getDomainForBrowser(gBrowser.selectedBrowser);
 +  let firstPartyDomain = getDomainForBrowser(gBrowser);
 }}}
 ? In `utils.js` it says
 {{{
 // Assuming this is called with gBrowser.selectedBrowser
 var getDomainForBrowser = (browser) => {
 }}}
 Please remove the superfluous whitespaces in
 {{{
 +    // So we can use it in boolean expressions to determine where the
 }}}
 and
 {{{
 +function torbutton_get_property_string(propertyname)
 +{
 +    try {
 }}}
 (the `try` line)

 commit 38720bfe8f88248534d4d211a40274c613a6ade6:

 1) Do we really need the remaining `k_tb_browser_update_needed_pref` parts
 (including the pref) now that the pref observer is gone (and the code
 acting upon pref changes)? What about the respective button CSS parts?
 Can't we get rid of them as well in this commit? For instance, if we don't
 set `tbStatus` anymore it seems to me we don't need the `[tb-status="on"]`
 and `[tb-status="off"]` parts anymore either etc.

 2) Please adapt the commit message to what has happened since you wrote
 it. :)

 Please base your new branch on current `master`.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28745#comment:20>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online