[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #25750 [Applications/Tor Launcher]: update Tor Launcher for ESR 60



#25750: update Tor Launcher for ESR 60
--------------------------------------------+------------------------------
 Reporter:  mcs                             |          Owner:  brade
     Type:  defect                          |         Status:
                                            |  needs_revision
 Priority:  Very High                       |      Milestone:
Component:  Applications/Tor Launcher       |        Version:
 Severity:  Normal                          |     Resolution:
 Keywords:  ff60-esr, TorBrowserTeam201805  |  Actual Points:
Parent ID:                                  |         Points:
 Reviewer:                                  |        Sponsor:
--------------------------------------------+------------------------------

Comment (by sysrqb):

 Replying to [comment:33 gk]:
 > Some comments (just nits):
 >
 > commit 785568d05b12f819a29b90c2dcd4e55b821a2047:
 > Any reason why the `minVersion` for Fennec is 45 and not 52? (I am fine
 with leaving the patch you have given that we are not really enforcing 52
 for desktop either, just curious)

 No, I don't remember why I chose that. Good question.

 >
 > commit 2e1e760a8393de281318e97ef44b2e89ba67879c
 > "Gecko now requires "0o"-prefixed octal literals" <- Are you sure about
 that? Yes, the warning shows up in the browser console but the bug you are
 citing is already fixed in Firefox 48, yet Tor Browser 7 does not show the
 warning. Fixing the octals is good, though. I hunted a bit but finding the
 actual bug behind this change seems a bit tricky. I think we could just
 say "Fix deprecated octal literals" in the commit message and move on.

 The runtime now throws a syntax error:

 {{{
 JavaScript error:
 jar:file:///home/user/firefox/TorBrowser/Data/Browser/profile.default/extensions
 /tor-launcher@xxxxxxxxxxxxxxxxxx!/components/tl-process.js, line 1003:
 SyntaxError: "0"-prefixed octal
  literals and octal escape sequences are deprecated; for octal literals
 use the "0o" prefix instead
 }}}

 But, I see 52ESR should throw the same error at runtime, so I'm not sure
 why TorLauncher and TorButton currently work.

 >
 > You are using "Bug XXXXXXX" and "bug XXXXXXX" for referencing Mozilla
 bugs within a sentence. I think you should stick to one format and the
 latter is the better one.

 Agreed, I'll change this.

 >
 > commit 039bd44ce1a65bbc7bcacfa7a6b114b744a84b8f
 >
 > s/var loader/let loader/
 > {{{
 > +      TorLauncherLogger.log(5,"Ignoring invalid pref ending with a
 period: '" +
 > }}}
 > Whitspace between "," and "\"".
 >

 I did this so the line length is less than 80 chars. If you prefer adding
 the space then we can move the format string onto the next line.

 > You want to have pairwise "'" but are forgetting sometimes the closing
 one.

 I'll review this and confirm there are matching quotes.

 >
 > commit 3f2936d1323c36d1882b81ab155bf9fd48e27a37
 >
 > The indentation of the new code block is off by one.

 I'll change this.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25750#comment:35>
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