Opened 3 years ago

Closed 3 years ago

#14631 closed defect (fixed)

Users that try to run from DMG files run into "Another copy of Firefox is running"

Reported by: arthuredelstein Owned by: mcs
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-usability, uxsprint2015, tbb-usability-stoppoint-app, tbb-helpdesk-frequent, tbb-firefox-patch, tbb-4.5-alpha, TorBrowserTeam201503R, MikePerry201503R
Cc: gk, brade, mcs, mikeperry Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Somehow we should figure out how to avoid this bug. Is it possible to write Firefox Profile files in /var/tmp or maybe not write them at all?

Child Tickets

Change History (19)

comment:1 Changed 3 years ago by arthuredelstein

(This is for OS X.)

comment:2 Changed 3 years ago by mikeperry

Keywords: tbb-usability-stoppoint-app added

comment:3 Changed 3 years ago by gk

Cc: gk added

comment:4 Changed 3 years ago by gk

What is causing this?

comment:5 in reply to:  4 Changed 3 years ago by mcs

Cc: brade mcs added

Replying to gk:

What is causing this?

I assume that the DMG images we create use a read only file system. This is not a problem for most Mac applications, but since Tor Browser needs to write to the Firefox profile Bad Things Happen.

My guess is that it will be extremely difficult to prevent Firefox from writing to the profile (since there are so many components within Firefox that assume they can do so). It might be better to just improve the error message to read something like:

You cannot run Tor Browser from a read-only filesystem.
Please copy Tor Browser to your Desktop or Applications folder before trying to use it.

comment:6 Changed 3 years ago by mikeperry

Yes, I think fixing the error message is the right approach here. In fact, I was tempted to call this a dup of #4782, but people at the UX sprint also specifically suggested trying to tailor the error message to the DMG use case. However, I think your error message above probably captures the major cases where the error may happen.

Feel free to dup this to #4782 if you agree (or dup #4782 to this bug if you feel that #4782 is too noisy and disorganized).

comment:7 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201502 added

Actually, I would like to see us try to get the error message for this into 4.5a4 if it is easy.

comment:8 Changed 3 years ago by mcs

Status: newneeds_information

OK, there is good news and there is bad news. The good news is that Kathy and I tracked down the spot in the code where we can detect failure due to the profile being on a read-only file system (inside nsProfileLock::LockWithFcntl()). And we can propagate a new nsresult out and pass it into nsAppRunner.cpp's ProfileLockedDialog() function, and so we can display a unique error message to cover this situation.

The bad news is that, since the profile has not yet been loaded, we cannot access Torbutton's string bundle... which is where we would typically put a new localizable string for the new error message. Here are a few options:

  1. Use a hard-coded English string.
  2. Start including new localized strings in Tor Browser somehow. This might be difficult to manage with our existing Transifex-based process. I think we should avoid going down this path.
  3. Resolve this ticket as "won't fix" and hope that efforts we make for #14687 will keep users out of trouble.
  4. Your idea here.

comment:9 in reply to:  8 Changed 3 years ago by mikeperry

Status: needs_informationnew

Replying to mcs:

OK, there is good news and there is bad news. The good news is that Kathy and I tracked down the spot in the code where we can detect failure due to the profile being on a read-only file system (inside nsProfileLock::LockWithFcntl()). And we can propagate a new nsresult out and pass it into nsAppRunner.cpp's ProfileLockedDialog() function, and so we can display a unique error message to cover this situation.

The bad news is that, since the profile has not yet been loaded, we cannot access Torbutton's string bundle... which is where we would typically put a new localizable string for the new error message. Here are a few options:

  1. Use a hard-coded English string.
  2. Start including new localized strings in Tor Browser somehow. This might be difficult to manage with our existing Transifex-based process. I think we should avoid going down this path.
  3. Resolve this ticket as "won't fix" and hope that efforts we make for #14687 will keep users out of trouble.
  4. Your idea here.

This is unfortunate. However, all hope is not lost. With some disgusting hacks, we should still be able to access torbutton.properties strings at this point. Basically, we need to construct a URI like: jar:file:///CWD/Browser/TorBrowser/Data/Browser/profile.default/extensions/torbutton@torproject.org.xpi!/chrome/locale/LOCALE/torbutton.properties, where some code gets the current working directory of the browser and does some magic to determine the right locale directory and sets CWD and LOCALE appropriately.

Once we have this jar URI for the current directory and current locale, we should be able to pass that jar URI into https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIStringBundleService#createBundle%28%29. You can then extract the strings as per the normal string bundle APIs and shove them into the error dialog.

(Note also that setting Torbutton LOCALE directory properly also seems to be slightly tricky, as the Torbutton locale directories do not appear to cleanly match the current Firefox general.useragent.locale pref values. Torbutton omits country codes in some but not all cases, but there must be some code that does this conversion already, as it works for Firefox's build-in localization currently, but I am not sure where this code is).

This hack should be neatly abstracted from the rest of the patch, as I suspect that a generic "This profile directory cannot be written to" error message fix is something that Mozilla would take, if they don't flee in terror upon the sight of this localization hack.

Is this too insane? I think given that the alternative is to somehow rebuild all of the Mozilla language pack XPIs just to pick up this new resource, this might be cleaner. Or maybe not. We're starting to run into quite a few situations where we're referencing Torbutton strings in the browser, so perhaps deploying a proper localization solution for the browser is something we should do..

Either way, I think given the frequency of this issue (as per frequent helpdesk reports about #4782), we should invest the effort in some kind of localized and user-intelligible error message here, though.

comment:10 Changed 3 years ago by mikeperry

Note also that there are several other cases where users can run into this cryptic error condition aside from just running TBB from the DMG. Our portable app model really causes this to happen in a lot of cases. #4894 is one such case that probably happens on all three OS platforms, and I imagine there are lots of cases where people run into #4782 after copying TBB to a USB drive with improper ownership/permissions, or when simply trying to write-protect their TBB install for extra security when using it on untrusted computers.

It's really unfortunate our portable app model prevents these likely fairly common use cases from working, and the least we can do is explain the situation to the user as best we can, when it happens.

comment:11 Changed 3 years ago by mikeperry

Keywords: tbb-4.5-alpha added

comment:12 Changed 3 years ago by mcs

Cc: mikeperry added
Owner: changed from tbb-team to mcs
Status: newassigned

comment:13 Changed 3 years ago by mcs

Keywords: TorBrowserTeam201502R added; TorBrowserTeam201502 removed
Status: assignedneeds_review

Patch 1 is available here:
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug14631-01&id=bebc69b8e8e919abec272f9cff6ab13fb9ea51f7

This adds two new error messages (one of which has a Mac OS specific variant):

Tor Browser Profile Problem

Tor Browser does not have permission to access the profile.
Please adjust your file system permissions and try again.

Linux and Windows:

Tor Browser Profile Problem

You cannot run Tor Browser from a read-only file system.  Please copy
Tor Browser to another location before trying to use it.

Mac OS:

Tor Browser Profile Problem

You cannot run Tor Browser from a read-only file system.  Please copy
Tor Browser to your Desktop or Applications folder before trying to use it.

Actually, currently all of the messages say "Firefox" instead of "Tor Browser" because MOZ_APP_DISPLAYNAME is not available inside nsAppRunner.cpp. We can fix that by hard-coding "Tor Browser" when we do the next patch for this ticket (which will pull the new strings out of Torbutton to allow for localization) or we can fix it by addressing #14977 (a riskier solution).

comment:15 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201503R added; TorBrowserTeam201502R removed

comment:16 Changed 3 years ago by mcs

Keywords: tbb-helpdesk-frequent tbb-firefox-patch added

I marked #4782 as a duplicate. Transferring some keywords.

comment:17 Changed 3 years ago by mcs

Also see #4894 (now closed as a duplicate).

comment:18 Changed 3 years ago by mikeperry

Keywords: MikePerry201503R added

comment:19 Changed 3 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed

Ok, this looks good to me (though man, getter_Copies() is a scary pattern). Merged!

Thanks a lot for doing this. I think it will help a lot of cases that currently prevent people from using TBB. I am wondering if we may end up eliminating profileAccessDenied, though. It seems a bit cryptic still. I suspect that it will be *mostly* #4894 that hits that string (but only if another user is actually currently running a copy of Tor Browser), but I suppose there may be other cases, too, so a general string may be useful until we get more data on exactly how people tend to end up in that situation.

Note: See TracTickets for help on using tickets.