Re: [tor-bugs] #26100 [Applications/Tor Browser]: Update Tor Button for ESR 60

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#26100: Update Tor Button for ESR 60
-------------------------------------------------+-------------------------
 Reporter:  igt0                                 |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ff60-esr, tbb-torbutton,             |  Actual Points:
  TorBrowserTeam201805                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by igt0):

 Replying to [comment:8 gk]:
 > Okay, this works for me, nice! Here comes a first batch of review
 comments (sorry, I have to split this up as I must switch context but
 wanted you to give some feedback to already work on):
 >
 > commit 1eafafefd2dfb6006f9f2686e6ecb591da8bc434
 >
 > Adding a hint to the underlying Mozilla bug
 (https://bugzilla.mozilla.org/show_bug.cgi?id=1414390) to the commit
 message would be good I think.

 OK!

 >
 >
 > commit 629f58cfffefb3f58ceab7275e0f1e1194a6f012
 >
 > Is that new that `catch` statements can't have `if` statements anymore?

 Yes, conditional clause was removed.

 In the page https://developer.mozilla.org/en-
 US/docs/Web/JavaScript/Reference/Statements/try...catch
 it says:

 Reminder: this functionality is not part of the ECMAScript specification
 and has been removed in Firefox 59. It's not supported in any current
 browser anymore.



 >
 >
 > commit 9c3e58597f3f3c41af2d44ae809a652c3a7d91a7
 >
 > Could you move the moz- removals in this bug to the previous commit?
 >
 > I don't understand your `plugin.disable` related commit/code change.
 It's not set by Mozilla code in ESR52 either. We are setting it in our own
 pref file we ship with the browser. Thus, it seems to me setting it here
 is not necessary?
 >
 > What speaks against using `getBrowser()`? It's cheaper to use
 `window.gBrowser`?
 >
 > Starting with "1." in the commit message without a "2." seems weird. :)
 Could you add more stuff as you seemed to want or reword that part?
 >
 >
 > commit ec0e146668a817954ee75b78216151830e626ebc
 >
 > Please do a
 > {{{
 > const Cu = Components.utils;
 > }}}
 > and use `Cu` in `cookie-jar-selector.js`
 > {{{
 > +  //Services.console.logStringMessage(`getPrefValue(${prefName})`);
 > }}}
 > No need for this commented out log statement
 > {{{
 > +       this.logmethod = 2;
 > }}}
 > Why is that needed?
 >
 > There are two additional new lines in `startup-observer.js` which are
 not needed.
 >
 > Re the pref loading:
 >
 > 1) Speaks anything against having that in utils.js instead of creating a
 new module?
 >
 > 2) I think we should set the prefs on the default prefs branch as it is
 done right now. That allows the user easily see which preferences got
 modified by themselves/the browser and more importantly there is the
 option to return to save defaults. tor-launcher's
 949379555010a04633c64b08ed52896a86c2cce6 might give some inspiration.

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

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs