[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #10760 [Applications/Tor Browser]: Integrate TorButton to TorBrowser core to prevent users from disabling it

#10760: Integrate TorButton to TorBrowser core to prevent users from disabling it
-------------------------------------------------+-------------------------
 Reporter:  Rezonansowy                          |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  AffectsTails, tbb-parity, ux-team,   |  Actual Points:
  TorBrowserTeam201906, GeorgKoppen201906        |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by gk):

 * keywords:
     AffectsTails, tbb-parity, ux-team, TorBrowserTeam201906R,
     TorBrowserTeam201906, GeorgKoppen201906
     =>
     AffectsTails, tbb-parity, ux-team, TorBrowserTeam201906,
     GeorgKoppen201906
 * status:  new => needs_revision


Comment:

 Replying to [comment:53 acat]:
 > > with the corresponding ones from DTD in securityLevel.js
 (​https://github.com/acatarineu/tor-browser/commit/30429+1).
 > Sorry, this was not the right commit implementing this. The duplicated
 translations removal is https://github.com/acatarineu/tor-
 browser/commit/6647e87dc1ed59c5b39e8618bf8753d1d8423343, and the rest of
 the torbutton work in tor-browser is in https://github.com/acatarineu/tor-
 browser/commit/8f841e5ccd0af5b1f73f5f9c74bb0fd866ba4c33.
 >
 > The torbutton branch with the changes is
 https://github.com/acatarineu/torbutton/commits/10760.

 Let's start with the Torbutton changes here:

 2a501682217eff74df47f45c60618b71f2a38353 -- please add the dependent
 changes to other files to this commit as well so it is self-contained as
 much as possible (there are still mentions of `torcookiedialog.xul`,
 `popup.xul` etc. in other files)

 18e7226776301a82bb2c5715739b25f9c2b281ad -- There are several things to
 fix up/think through, in no particular order:

 1) What is the reason for commenting out code in the patch? If we don't
 need it then we should remove it. Additionally, the `torbutton-new-
 identity` part is even disabled only sometimes but it's not obvious why
 not every time.

 2) In `torbutton_utils.js` there is `Services` used, but `// let {
 Services }`

 3) In aboutTor.js (and `cookie-jar-selector.js` + `domain-isolator.js` +
 `dragDropFilter.js` + `external-app-blocker.js` + `torCheckService.js` +
 `torbutton-logger.js` + partly `tor-control-port.js` + `utils.js`)
 we have
 {{{
 const {XPCOMUtils}
 const {Services}
 }}}
 while having spaces after and before the curly braces elsewhere in the
 same changes, please stick to one style.

 4) nsIObserverService in cookie-jar-selector.js -> Services.obs

 5) `+  QueryInterface: ChromeUtils.generateQI([Ci.nsISupports,
 Ci.nsIClassInfo]),`

 Why suddenly here `Ci.nsISupports` but a lot of other `generateQI()` calls
 don't have it? There is more of this in other components, like `domain-
 isolator.js`, `dragDropFilter.js`, `external-app-blocker.js`,
 `torCheckService.js`, `torbutton-logger.js`

 6) s/mecanism/mechanism/ in `noscript-control.js`

 7) Why do we need to resort to `broadCastAsyncMessage` and
 `sendAsyncMessage` would not be enough?

 8) There is an `addTab` call in `menu-overlay.xul` that needs to get
 adjusted

 9)
 {{{
             eventSuppressor = browser.contentWindow.
                 QueryInterface(Ci.nsIInterfaceRequestor).
                        getInterface(Ci.nsIDOMWindowUtils);
 }}}
 We should use `windowUtils` here as well, no? Not sure whether we can just
 take `m_tb_domWindowUtils` or actually need the content window version,
 though.

 8) It seems you missed to adapt `startup-observer.js` and doing all the
 clean-up there, too?

 9) In `aboutTor-content.js` replace
 {{{
 let brandBundle = Cc["@mozilla.org/intl/stringbundle;1"]
                           .getService(Ci.nsIStringBundleService)
 }}}
 with `Services.strings`?

 10) `new Array()` in `let tabsToRemove = new Array();` in torbutton.js
 should get changed as well.

 11) There are `nsICookieManager2` calls in `cookie-jar-selector.js` which
 could get replaced by `Services.cookies`.

 Marking this as `needs_revision` while I look over the other commits.

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