[tbb-bugs] #10760 [Applications/Tor Browser]: Integrate TorButton to TorBrowser core to prevent users from disabling it
Tor Bug Tracker & Wiki
blackhole at torproject.org
Wed Jul 31 11:05:04 UTC 2019
#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:
GeorgKoppen201907, TorBrowserTeam201907, |
tbb-9.0-must-nightly |
Parent ID: | Points:
Reviewer: | Sponsor:
-------------------------------------------------+-------------------------
Comment (by gk):
Replying to [comment:67 acat]:
> I addressed the comments in a new branch: https://github.com/acatarineu
/tor-browser/compare/30429+1..30429+2.
>
> I rebased the previous branch, fixing up the torbutton commit and
squashing the circuit UI commit. Also realized that I had done the locale
deduplication changes as a fixup for the torbutton integration commit, but
> I think it should have been for the security UI one. So I moved those
changes to the security UI commit (Bug 25658). Also included a couple of
changes not mentioned in the review: removed redundant torbutton in
`toolkit/moz.build` and removed `Example *` from `identityPanel.inc.xul`.
Removing `example A` etc. makes me a bit nervous. It got introduced in
#15086 back then. I am not sure whether that part has been essential on
solving the RTL issues but we should double-check that we don't have weird
regressions, in particular as all the other code surrounding the `<li>`
elements, like styling is left untouched.
[snip]
> > Where does it remove the duplicated translations in that commit?
> I meant that the duplicated strings from properties are not used anymore
in that commit (and therefore can be removed). But did not remove them
from the translation files yet.
>
> > The general approach looks good to me, nice find. I think we should
get rid of all getString() calls while we are at it and make sure that
everything we need is available both for desktop and mobile, hence in the
.dtd file (we have #24653 for the localization parity part which could be
solved while redoing this part).
> So, if I understand it correctly, we should remove
securityLevel.properties and move the non-duplicated ones to
torbutton.dtd. Should we adapt the keys like:
`securityLevel.safest.tooltip` -> `torbutton.prefs.sec_safest_tooltip`?
Should I do a translation repository patch for this and review it here?
Yes, regarding your first and second question. I think there is no need
for a translation repo patch. Just do the patch in Torbutton and it will
propagate once someone commits the changes to `master`. The patch could be
in #24653 which could be on top of the general #10760 patch for review. We
can squash that one in a later rebasing then if we think that's useful.
(If you go that route please make #24653 a child bug of this ticket so we
don't lose track here)
[snip]
> > We should think as well more general about a mechanism of avoiding
duplicated translations as I am not sure whether the hack you found is
applicable in more than the sec-settings situation.
> Do you have in mind specific cases where it would not work? I think from
privileged code we will always be able to create a domparser to get the
translation (actually, currently also from non-privileged web content).
But in any case, I think Fluent is the way to go for this, a single format
that will work for programatic and "document" localization without these
hacks.
Fair enough and, no, I don't have specific cases in mind right now.
> > using a NullPrincipal for new tabs seems indeed to be a good idea;
Would we get away with that for about:tor as well given that we try hard
to give it only content privs with the nsIAboutModule
> I did not find a way to open it with nullprincipal. I also tried other
about:*, and only `about:home`, `about:newtab` and `about:welcome` worked
for me (maybe it's because they have special handling in
`AboutRedirector::NewChannel`?).
I see. Let's go with what we have for now then.
We are close here. :)
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/10760#comment:74>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tbb-bugs
mailing list