[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_review
 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:24 mcs]:
 > Replying to [comment:23 sysrqb]:
 > > Although I agree with what I said, it's a terrible UX for users, so we
 should tell the user (somehow) that Tor Browser is in an undefined state.
 The only other option I see is we get rid of prefs.js entirely and somehow
 make sure all get...Prefs() calls use the correct default prefs.
 >
 > I think this situation is rare enough that we should not spend a lot of
 time trying to make it better from a UX perspective. Users should not edit
 the prefs.js file that is embedded in Tor Launcher (which is the most
 likely reason that loading the default prefs might fail).
 >
 > > Similar to what was mentioned above. I worry about this situation, but
 as long as we're careful about keeping prefs.js and the getIntPrefs()
 calls synchronized, this shouldn't be a problem.
 >
 > Kathy and I agree.
 >

 Great, thanks! I added a commit that synchronizes a few get..Pref() calls
 default values. I don't think we need all of them in sync, but it seems
 safer if retrieving a pref during initialization uses the same default
 value as defined in prefs.js.

 > > Right, so we have a few options and available formats, from what I
 see. The built-in formats do not play nicely with appending the
 milliseconds at the end, because some locales use 12-hour clocks and have
 AM/PM suffix, plus some add the timezone. We'd need additional logic for
 detecting this. The four built-in formats for en-US are (with milliseconds
 appended):
 > >
 >
 > Kathy and I spent a little time looking for an alternative. It seems we
 have more control if we use JavaScript's built-in Intl.DateTimeFormat
 object, i.e.,
 > {{{
 > let options = { year: '2-digit', month: 'numeric', day: 'numeric',
 >                 hour: 'numeric', minute: 'numeric', second: 'numeric',
 >                 hour12: false };
 > let timeStr = new Intl.DateTimeFormat('en-US', options)
 >                       .format(logObj.date, options);
 > }}}
 >

 Thanks, that looks good. The only difference I see is this format inserts
 a comma between the date and time.

 {{{
 5/15/18, 23:20:43.525 [NOTICE] Bootstrapped 100%: Done
 }}}

 Replying to [comment:25 mcs]:
 > For 0219dcd1aaef1a5460bec39cdd95742222ff9c08, a couple of "leftover"
 functions are defined in `modules/tl-util.jsm` but they are not used;
 please remove them:
 > * `_defaultPrefsLoaded()`
 > * `loadDefaultPreferencesAndThrow()`

 Done.

 I pushed `bug25750_3` for review. Thanks.

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