[tbb-bugs] #13671 [Tor Browser]: Circuit display is broken if bridges are being used.

[Tor Bug Tracker & Wiki]

#13671: Circuit display is broken if bridges are being used.
-------------------------+-------------------------------------------------
     Reporter:  yawning  |      Owner:  tbb-team
         Type:  defect   |     Status:  needs_review
     Priority:  normal   |  Milestone:
    Component:  Tor      |    Version:
  Browser                |   Keywords:  TorBrowserTeam201412R,
   Resolution:           |  TorBrowserTeam201412
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+-------------------------------------------------

Comment (by arthuredelstein):

 Replying to [comment:21 mcs]:
 > Kathy and I reviewed 0001-13671-fix-circuit-display-when-bridges-are-
 used.3.patch.  It was interesting to learn about Task.jsm; it seems like a
 good fit for this code and it makes error handling much cleaner.  Here are
 our comments, all of which can probably be deferred (maybe file a new
 ticket?):

 Thanks for the review!

 > 1) Nit: s/nodeData/nodeDataForID/ in the Example comment here:
 > {{{
 > // nodeDataForID(controller, id)__.
 > // Returns the type, IP and country code of a node with given ID.
 > // Example: `nodeData(controller,
 "20BC91DC525C3DC9974B29FBEAB51230DE024C44")`
 > // => `{ type : "default" , ip : "12.23.34.45" , countryCode : "fr" }`
 > let nodeDataForID = function* (controller, id) {
 >   let result = {}; // ip, type, countryCode;
 > }}}

 Fixed.

 > 2) Nit: Remove spaces before commas in the code above and within
 info.routerStatusParser inside tor-control-port.js.

 Fixed.

 > 3) Nit: In the code above, change the "let result = {};" line to:
 > {{{
 > let result = {}; // type, ip, countryCode;
 > }}}
 > (so that the properties mentioned in the comment are listed in the same
 order as in the preceding comments).

 Fixed.

 > 4) Inside the nodeLines function, "Unknown country" and other nearby
 strings should be localized, someday.

 Good point. New ticket: #13881.

 > 5) Nit: In this code (now switching to tor-control-port.js), remove the
 "and " before dispatcher.removeCallback (there are now two "and"s in one
 sentence).
 > {{{
 > // __io.callbackDispatcher()__.
 > // Returns dispatcher object with three member functions:
 > // dispatcher.addCallback(regex, callback), and
 dispatcher.removeCallback(callback),
 > // and dispatcher.pushMessage(message).
 > }}}

 Fixed.

 > 6) Why do you use "getinfo config-text" instead of "getconf bridge"?

 Oops -- I have no idea why. New ticket: #13882.

 > 7) We saw an "addTabsProgressListener is not defined" exception on the
 Error Console at this line within syncDisplayWithSelectedTab():
 > {{{
 > gBrowser.addTabsProgressListener(listener2);
 > }}}
 > But we were not able to reproduce the problem.  Our best guess is that
 some combination of multiple tabs and New Identity triggered the problem.
 Looking at the code, it seems like the only way to get there would be if
 start() within setupDisplay() failed.  If gBrowser is missing functions
 then the window it is part of must be in the process of closing or already
 closed.

 I tried a little but wasn't able to get that exception to happen. We could
 check for the presence of gBrowser.addTabsProgressListener at runtime, but
 maybe it's better to try to understand the problem first. It seems like
 this bug might happen in a new window if there were s a race condition
 between constructing the gBrowser object and running
 createTorCircuitDisplay().

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