Closed Bug 1494436 Opened 5 years ago Closed 11 months ago

[Linux][Nightly] I can no longer open external links in Nightly after the 2nd selection of the "Use Nightly as my default browser"

Categories

(Firefox :: Shell Integration, defect, P3)

Product:
Firefox
Component:
Shell Integration
Version:
64 Branch
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
101 Branch
Tracking Flags:
Tracking Status
firefox64 --- wontfix
firefox101 --- fixed

People

(Reporter: paul.boiciuc, Assigned: smcv, NeedInfo)

References

Details

Attachments

(2 files, 1 obsolete file)

Short video of the issue
101 bytes, text/plain
Details
Bug-1494436-Unset-MOZ_APP_LAUNCHER-for-external-MIME-hand.patch
3.35 KB, patch
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:64) Gecko/20100101 Firefox/64
Build ID: 20180926132058 / Version: 64.0a1

Steps to reproduce:

1. Open Nightly and select the "Use Nightly as my default browser" when prompted.
2. Close the browser.
3. Navigate to Thunderbird and select an external link.
4. After the selected link is opened in Nightly without any issues, select the same option as in step 1 when prompted.
5. Navigate back to Thunderbird and select a link again.
6. Close the browser and repeat step 5. 

Actual result: 

The links are no longer opened starting with the 2nd selection of the "Use Nightly as my default browser". Once the browser is closed, it can no longer be opened by selecting a link. It can be opened only from the .exe file. I have attached a file with a short video for better understanding.

Expected result: 

The links are opened without any issues in Nightly as long as this is the default selected browser.
Priority: -- → P2

I can reproduce this with Ubuntu's Thunderbird 68.2.2 and Nightly 75. If we are launched by clicking a link in Thunderbird, clicking "Use as my default browser" actually sets the xdg default-web-browser to thunderbird.desktop, and then Thunderbird refuses to open those links (I see a "https://www.mozilla.org" is not a valid feed." message in Thunderbird's status bar).

This is because when Nightly is launched from Thunderbird, the MOZ_APP_LAUNCHER environment variable is still pointing at Thunderbird. The shell service uses that to decide mAppPath, so any further work we do with checking or setting ourselves as a handler will use Thunderbird's path and app info.

There's a few other places that use this as well, and I think it's going to mess up at least the crash reporter.

Debian's Thunderbird sets this in the wrapper.sh, I think in order to make sure that the wrapper is used to launch instead of the binary.

Probably the best thing to do would be for Thunderbird to clear this var when it shells out. I think that this could be done by passing a context to g_app_info_launch_uris(), and it's possible to use g_app_launch_context_unsetenv(), but I'm not sure where to get a context from. I don't know enough gnome to follow this any further at the moment.

but I'm not sure where to get a context from

Using g_app_launch_context_new:

GAppLaunchContext *ctx = g_app_launch_context_new();
if (!ctx) return NS_ERROR_FAILURE;
g_app_launch_context_unsetenv(ctx, "MOZ_APP_NAME");
g_app_launch_context_unsetenv(ctx, "MOZ_APP_LAUNCHER");
g_app_launch_context_unsetenv(ctx, "MOZ_LIBDIR"); // This too?

This context should be passed to the g_app_info_launch_default_for_uri and g_app_info_launch_uris calls.

Flags: needinfo?(agashlin)

(In reply to bitter.taste from comment #4)

Using g_app_launch_context_new:

This context should be passed to the g_app_info_launch_default_for_uri and g_app_info_launch_uris calls.

Thanks for the pointers! I'm setting the priority to indicate it's fallen into the backlog, but it would be good to fix here (with tests) so Thunderbird can pick it up from the common Toolkit.

Flags: needinfo?(agashlin)
Severity: normal → S3
Priority: P2 → P3

Here is a tested patch based on the comments above: https://people.debian.org/~smcv/Bug-1494436-Unset-MOZ_APP_LAUNCHER-for-external-MIME-hand.patch

Sorry, I am not in a position to write automated tests for this (or even to know whether it's feasible to do so), but it has the desired result when applied as a patch to Debian's thunderbird package, version 1:91.6.2-1. I tested with Debian's firefox-esr package as the web browser, but newer Firefox branches probably have similar behaviour.

Steps to reproduce the issue in a test account:

  • use an expendable user ID or a VM, because these test steps are destructive
  • Debian testing/unstable system (I used a virtual machine with GNOME)
  • firefox-esr as default web browser (this is the default in Debian GNOME)
  • rm -f ~/.config/mimeapps.list
  • Completely exit from both Firefox and Thunderbird
  • strace -eexecve -ff thunderbird
  • no need to set up an actual email account
  • press Alt, Help -> About Thunderbird, click on Privacy Policy
    • it launches firefox as a subprocess (you'll see this in the strace)
  • If Firefox asks to be made the default browser:
    • click "Make Firefox my default browser"
  • Completely exit from both Firefox and Thunderbird again
  • cat ~/.config/mimeapps.list
  • xdg-open http://example.com

Expected result: mimeapps.list doesn't exist, is empty, or lists firefox-esr as default web browser. example.com opens in Firefox.

Actual result: mimeapps.list has thunderbird.desktop as default for HTML, HTTP etc., and Thunderbird opens instead of Firefox.

Mitigation: If firefox-esr is already running before you click the link in Thunderbird, then the firefox-esr subprocess of Thunderbird just sends an IPC request to the existing firefox-esr instance and then exits, so people who habitually have a web browser open at all times will often not experience this bug (which is presumably why Debian's Thunderbird maintainers didn't experience this).

Sorry, the previously-attached patch was an outdated version. Try this one instead.

Attachment #9267524 - Attachment is obsolete: true

I don't really know enough about what's going on here to say much about this patch with any confidence.

@jhorak - It looks like you wrote most of this file, would you be available to look at this patch?

Flags: needinfo?(jhorak)

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #9)

I don't really know enough about what's going on here to say much about this patch with any confidence.

@jhorak - It looks like you wrote most of this file, would you be available to look at this patch?

Emilio, I see you've been in this code fairly recently as well -- any chance you're comfortable reviewing this?

Flags: needinfo?(emilio)

Yes, the patch looks reasonable to me, it is harmless at best. I have some nits that I have addressed locally, but I can't put myself as the reviewer of the revision because Phabricator thinks I authored it. So I think I'll do the manual push dance for this one.

Flags: needinfo?(jhorak)
Flags: needinfo?(emilio)
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/58d678ccbab1
Unset MOZ_APP_LAUNCHER for external MIME handlers. r=emilio

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

Yes, the patch looks reasonable to me, it is harmless at best. I have some nits that I have addressed locally, but I can't put myself as the reviewer of the revision because Phabricator thinks I authored it. So I think I'll do the manual push dance for this one.

Thanks!

https://hg.mozilla.org/mozilla-central/rev/58d678ccbab1

Status: NEW → RESOLVED
Closed: 11 months ago
status-firefox101: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch
Assignee: nobody → smcv

Thanks for adjusting and applying this.

  • gboolean result = g_app_info_launch_uris(
  •  mApp, &uris, GetLaunchContext().get(), getter_Transfers(error));

Just to be sure: is RefPtr "smart" enough that it will ensure the GObject reference to the GAppLaunchContext is not released until after the call to g_app_info_launch_uris(), even though there is no variable in-scope to own the reference?

Yes, C++ lifetime extension rules guarantee that, I'm pretty sure. I'm on my phone now but I can try to give a proper standard reference tomorrow.

I think the "Temporary object lifetime" section in https://en.cppreference.com/w/cpp/language/lifetime is probably a decent enough reference (I'd look the specific spec text but... :))

QA Whiteboard: [qa-101b-p2]

Reproduced this issue on an affected Nightly build from 2018-09-26, on Ubuntu 20.04.
The issue is still reproducing on the latest Nightly and the treeherder build from Comment 14.

Flags: needinfo?(smcv)

The bug has been fixed here in Toolkit code, so it needs to be picked up in Thunderbird. I don't think there's a bug for this on the Thunderbird side.

I'm not very familiar with Debian procedure, and I haven't tested this, it looks like Debian has picked up the patch, so it should be fixed in Thunderbird 1:91.9.0-1. You might try installing sid without updates and seeing if it reproduces (with the distribution's Firefox and Thunderbird), and then upgrading.

It looks like it should be fixed in an updated Debian oldstable (buster, 10) as the patch is in 1:91.9.0-1~deb10u1, but it's not clear to me that this update is actually released. The current stable (bullseye, 11) package page doesn't show even show 1:91.9.0-1~deb11u1 at all.

I expect that Ubuntu will pick this up sooner or later, but I'm even less familiar with their procedure, it doesn't seem present in 22.04.1's latest 1:91.9.1+build1-0ubuntu0.22.04.1.

(In reply to Adam Gashlin (he/him) [:agashlin] (ex-moco) from comment #20)

so it should be fixed in Thunderbird 1:91.9.0-1. You might try installing sid without updates and seeing if it reproduces (with the distribution's Firefox and Thunderbird), and then upgrading.

Quick correction to avoid more confusion: This patch was taken into Debian in 1:91.7.0-2, so you'd need a version older than that to reproduce.

And, I hadn't realized, there isn't really such a thing as "sid without updates". I think you could use snapshot.debian.org:

  • initially install with https://snapshot.debian.org/archive/debian/20220310T035035Z/ in sources.list, when the latest was 1:91.6.2-1
  • and then change that to https://snapshot.debian.org/archive/debian/20220316T155501Z/ and update to get 1:91.7.0-1

Sorry for all the messages. I'm not even sure if this will be strictly helpful as it isn't really testing the source here. This bug might be better in a Toolkit or Thunderbird component, tracking Firefox doesn't make too much sense (except when Firefox is launching another Mozilla app, so maybe this would also happen the other way, e.g. when setting Thunderbird as default when launching a mailto: link from Firefox?).

You need to log in before you can comment on or make changes to this bug.