GetUserDataDirectoryHome in nsXREDirProvider.cpp breaks Orfox

The GetUserDataDirectoryHome seems to break the page loading in Orfox. I'm going to attach the patch that I have for this to this ticket, but I think it could be cleaner, I'm not sure what you would require though.

added TorBrowserTeam201608R component::applications/tor browser owner::tbb-team priority::medium resolution::fixed severity::normal status::closed tbb-mobile type::defect labels

Trac:
nsXREDirProvider.patch

Trac:
Keywords: N/A deleted, TorBrowserTeam201607R added
Status: new to needs_review

Amogh says this is #9173 (closed) related.

Thanks for tracking this down and for providing a patch.

It looks like Orfox stores the browser profile data in the user's home directory (or wherever $HOME points to). If that is the case, then in the long run Orfox should be built with --enable-tor-browser-data-outside-app-dir (which causes TOR_BROWSER_DATA_OUTSIDE_APP_DIR to be defined; see #13252 (moved)). Unfortunately, the code we added for #13252 (moved) has only been tested on OSX so you may not want to use --enable-tor-browser-data-outside-app-dir yet.

Regardless of whether Orfox uses --enable-tor-browser-data-outside-app-dir or not, Kathy and I would like to use a revised patch that modifies TorBrowser_GetUserDataDir() (which is inside xpcom/io/TorFileUtils.cpp). We may also need to make a small patch to nsXREDirProvider::GetUserDataDirectoryHome() to avoid putting the data underneath a Data/Browser subdirectory within $HOME.

Do you want Kathy and me to provide an untested patch for you to try, or do you want to create a revised patch?

For example, if you do not build with --enable-tor-browser-data-outside-app-dir, you would want to insert a #elif defined(ANDROID) block before the following code in TorBrowser_GetUserDataDir():

#else
  // User data is embedded within the application directory (i.e.,
  // TOR_BROWSER_DATA_OUTSIDE_APP_DIR is not defined).
  nsresult rv = GetAppRootDir(aExeFile, getter_AddRefs(tbDataDir));
  NS_ENSURE_SUCCESS(rv, rv);
  rv = tbDataDir->AppendNative(NS_LITERAL_CSTRING("TorBrowser"));
  NS_ENSURE_SUCCESS(rv, rv);
#endif

Trac:
Keywords: TorBrowserTeam201607R deleted, TorBrowserTeam201607 added
Status: needs_review to needs_revision

We don't mind building with or without the --enable-tor-browser-data-outside-app-dir flag.

I think I will let you get this patched as I'm not sure what the best way to deal with it is.

I don't think that we require a patch right now, as I'm fine with using this one for builds we're trying to get out now. But it would be useful to have this fixed by the next time we do a rebase.

In the case of Android, each app is a user itself, and so the home is a path unique for the app, and that only the app can read and write to. There is no other way or place for the data to be safely stored.

Replying to amoghbl1:

I don't think that we require a patch right now, as I'm fine with using this one for builds we're trying to get out now. But it would be useful to have this fixed by the next time we do a rebase.

I am concerned that we will forget about this if we do not address it now. If Kathy and I create a patch, do you have time to try it (compile it and let us know if it works)?

Trac:
Status: needs_revision to needs_information

Yes, we should write a patch for that now-ish. We already forgot this once.

Trac:
Status: needs_information to assigned

Replying to mcs:

Replying to amoghbl1:

I don't think that we require a patch right now, as I'm fine with using this one for builds we're trying to get out now. But it would be useful to have this fixed by the next time we do a rebase.

I am concerned that we will forget about this if we do not address it now. If Kathy and I create a patch, do you have time to try it (compile it and let us know if it works)?

Sure, I will test it out once it's ready and verify whether it works or not.

Please try the following patch: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug19706-01&id=d86fa0e328b8ea6a07741b55da5cc1bebb669b88

Trac:
Keywords: TorBrowserTeam201607 deleted, TorBrowserTeam201607R added
Status: assigned to needs_review

Replying to mcs:

Please try the following patch: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug19706-01&id=d86fa0e328b8ea6a07741b55da5cc1bebb669b88

This patch works fine with our build. Thank you for it!

Old reviews for the "new" month.

Trac:
Keywords: TorBrowserTeam201607R deleted, TorBrowserTeam201608R added

Looks good to me. Arhur, could you have a look at it as well?

Trac:
Cc: mcs, brade, gk to mcs, brade, gk, arthuredelstein

Also looks good to me. Sorry for the delay!

Fixed on tor-browser-45.3.0esr-6.0-1 (commit 7c50120b217807b98a8a99d16c53b760d2119280) and tor-browser-45.3.0esr-6.5-1 (commit b8726ee81e6eca2e302e2ea85c0d09ad22be1ec5).

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

moved to tpo/applications/tor-browser#19706 (closed)