[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #13671 [Tor Browser]: Circuit display is broken if bridges are being used.
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs