[tor-bugs] #23261 [Applications/Tor Launcher]: implement configuration portion of new Tor Launcher UI

[Tor Bug Tracker & Wiki]

#23261: implement configuration portion of new Tor Launcher UI
--------------------------------------------+------------------------------
 Reporter:  mcs                             |          Owner:  brade
     Type:  defect                          |         Status:  needs_review
 Priority:  Medium                          |      Milestone:
Component:  Applications/Tor Launcher       |        Version:
 Severity:  Normal                          |     Resolution:
 Keywords:  ux-team, TorBrowserTeam201710R  |  Actual Points:
Parent ID:  #21951                          |         Points:
 Reviewer:                                  |        Sponsor:  Sponsor4
--------------------------------------------+------------------------------
Changes (by mcs):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:33 gk]:
 > Okay. I went over all three commits and they look good to me apart from
 minor issues. Great work! I'll post all my comments here to not have them
 scattered over different tickets.
 >
 > c6fd54b8ba36bf88630db446c68ba8d04ed9727a
 >
 > 1)`<vbox class="firstResponses" align="center" >` <- whitespace before
 ">"
 > 2) I was a bit confused by the mix of of " />" and "/>", the former
 seems to be for older code? Just as a note that's not something that needs
 to get fixed
 > 3) please fix `TODO2017` items or file new tickets

 We addressed all of the above issues. For consistency, we removed all of
 the extra spaces from the XUL near " />" and ">" (although some are done
 in the other patches). We also reinstated the "For assistance" text and
 inserted the help text that Linda proposed in comment:25 (which should be
 updated after we have a broader discussion about how to improve it).

 We also found and fixed a bug that caused PTs were near the end of the
 list of
 defaults bridges to not be shown in the UI (e.g., snowflake was missing on
 Mac), and we removed some old code that set up the label and access key
 for the built-in <dialog> help button (which we no longer use, since we
 now have two buttons).

 > 1ebd091f1e185ccf3ffaa739bf8ec232447ead8a
 >
 > 1) Here you are not doing the "var" -> "let" renaming as in the previous
 commit, any reason for that? (not necessarily something to fix)

 We fixed some more of these in functions that we touched (but not
 everywhere yet).

 > 2) no mixed {} if-else clauses (see `readTorSettings()`)

 Bad habits ;) Fixed.

 > ba78bfa36ebac398511ddde1ece70f82d202383a
 >
 > 1) no mixed {} if-else clauses (see `_configureDefaultBridges` in tl-
 process.js and `showAlert()` in tl-util.jsm)

 Fixed.

 > One bug I found (it's not new though):
 >
 > 1) Start with connecting directly.
 > 2) Open the Tor Launcher network settings in the browser window and
 check the firewall option
 > 3) Click okay and restart the browser
 > 4) Cancel normal start-up and configure bridges (obfs4)
 > 5) Check the Tor Launcher network settings in the browser window and the
 firewall option is now unchecked (which should not be the case)

 Good find. I created #24098.

 > I am still chasing two other bugs which I failed to trigger again, so
 more might come. :)

 Okay. I am sure there are more bugs. We do have a new branch to review
 (still three patches):
 https://gitweb.torproject.org/user/brade/tor-
 launcher.git/log/?h=bug23261-02

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