[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_revision
 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 gk):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:30 mcs]:
 > Replying to [comment:29 gk]:
 > > First part of the review. This only covers comparing the attached
 design spec with the layout implemented. Apart from nits which probably
 don't matter (slightly different "Quit" button positioning etc.):
 >
 > The position of the Quit button varies by platform due to our use of
 Mozilla's XUL wizard element. We could change it to be consistent across
 platforms, but it would be extra work and might make things less
 consistent with other parts of Tor Browser.

 Yeah, don't do that.

 > > 1) The design shows that `obfs4` is selected by default and that it is
 the recommended bridge type. Both is missing in the layout implemented but
 it seems worthwhile to have, no?
 >
 > We implemented v10 of Linda's design. I think you may be looking at v9.
 I will attach the v10 PDF to this ticket (it was sent by Linda to some of
 us via email on Sept 26th).

 Heh. It seems I have been one of them. Thanks for adding v10 to this
 ticket, so others have the latest design draft available as well.

 > > 2) Is there are ticket somewhere tracking the fancy icons and progress
 implementation as shown on page 9 and 10 of the design doc? If not, could
 you create one?
 >
 > I created #23971.

 Thanks.

 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

 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)
 2) no mixed {} if-else clauses (see `readTorSettings()`)

 ba78bfa36ebac398511ddde1ece70f82d202383a

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

 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)

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

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