[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #13252 [Tor Browser]: Tor Browser on OS X should not store data into the application bundle
#13252: Tor Browser on OS X should not store data into the application bundle
----------------------------------+--------------------------------
Reporter: torosx | Owner: mcs
Type: defect | Status: needs_revision
Priority: Medium | Milestone:
Component: Tor Browser | Version:
Severity: Normal | Resolution:
Keywords: TorBrowserTeam201603 | Actual Points:
Parent ID: #6540 | Points:
Reviewer: | Sponsor: None
----------------------------------+--------------------------------
Changes (by arthuredelstein):
* sponsor: => None
Comment:
I read through each of the three tor-browser patches carefully and I
didn't find any problems, although I am pretty unfamiliar with this part
of the codebase so I may not understand all of the ramifications. Very
nice work! One question I have for patch
4d8e33f4dca21923f3dfef4e740c3c01f395ec1e:
{{{
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -42,6 +42,11 @@ const PREF_MATCH_OS_LOCALE =
"intl.locale.matchOS";
const PREF_SELECTED_LOCALE = "general.useragent.locale";
const UNKNOWN_XPCOM_ABI = "unknownABI";
+#ifdef TOR_BROWSER_VERSION
+#expand const TOR_BROWSER_VERSION = __TOR_BROWSER_VERSION__;
+const PREF_EM_LAST_TORBROWSER_VERSION =
"extensions.lastTorBrowserVersion";
+#endif
+
const UPDATE_REQUEST_VERSION = 2;
const CATEGORY_UPDATE_PARAMS = "extension-update-params";
@@ -866,6 +871,30 @@ var AddonManagerInternal = {
this.validateBlocklist();
}
+#ifdef TOR_BROWSER_VERSION
+ // To ensure that extension override prefs are reinstalled into the
+ // user's profile after each update, set appChanged = true if the
+ // Mozilla app version has not changed but the Tor Browser version
+ // has changed.
+ let tbChanged = undefined;
+ try {
+ tbChanged = TOR_BROWSER_VERSION !=
+
Services.prefs.getCharPref(PREF_EM_LAST_TORBROWSER_VERSION);
+ }
+ catch (e) { }
+ if (tbChanged !== false) {
+ // Because PREF_EM_LAST_TORBROWSER_VERSION was not present in
older
+ // versions of Tor Browser, an app change is indicated when
tbChanged
+ // is undefined or true.
+ if (appChanged === false) {
+ appChanged = true;
+ }
+
+ Services.prefs.setCharPref(PREF_EM_LAST_TORBROWSER_VERSION,
+ TOR_BROWSER_VERSION);
+ }
+#endif
}}}
Is it true here that the TOR_BROWSER_VERSION preprocessor directive
already contains quotation marks? I noticed in some other places in a
previous patch Kathy and Mark used
{{{
+#ifdef TOR_BROWSER_VERSION
+# Add double-quotes back on (stripped by JarMaker.py).
+#expand const TOR_BROWSER_VERSION = "__TOR_BROWSER_VERSION__";
+#endif
}}}
but perhaps the quotes are not stripped on AddonManager.jsm?
One other observation: in b03f511d38631243fec0e6c5427d9a50e602a762 it
would be nice to have a doc comment at the head of the
`migrateOneTorBrowserDataDir` function describing what it does. I also
have difficulty parsing the comment inside that function:
{{{
+ // The destination directory exists, which is expected in the case of
+ // migration of the browser profile. Set the new directory aside and
set
+ // tmpDir to point to the new, temporary location.
}}}
Could you make it explicit why an existing destination directory is
expected in migrating the browser profile? And, should the next sentence
be something like `Set the old directory aside and set tmpDir to point to
its new temporary location`? (Sorry if I'm misunderstanding.)
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13252#comment:41>
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