1818194 - Tab does not open when clicking on link in external application and Firefox is closed

Status: RESOLVED WORKSFORME

(Core :: Widget: Cocoa, defect, P3)

Description

[Sören Hentzschel]

As explained in https://bugzilla.mozilla.org/show_bug.cgi?id=1600153#c67:

Apparently this has been fixed by Bug 1792466

I don't know if it's since bug 1792466 (that was fixed in Firefox 109 Nightly) but compared to Firefox 108 Beta the bevaviour is much worse in Firefox 109 Nightly.

Until Firefox 108 Beta Firefox used to open two windows: one windows with the last session and another one with the opened link. Now Firefox only starts the window with the last session. The URL from the link does not open at all, neither in a new window nor in a new tab. Clicking a link just opens Firefox and you have to press the link in the email a second time to open it in Firefox.

STR:

1. Make sure that Firefox is closed
2. Click a link in an external application, like Thunderbird

Expected:

Firefox starts with the last session and the clicked link opens in a new tab. Or at least the new tab should be opened in a new window as it was the case until Firefox 108.0.2. That's not perfect but at least the website opened.

Actual:

Firefox starts with the last session but the clicked link does not open at all until you click it again in the external application. I tested again and can confirm that the regression exists since Firefox 109.

Tested on macOS 13.2.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Daniel Bodea [:danibodea]]

I have managed to reproduce this issue on Mac OS 11 with Firefox Release v111.0, v110.0, 109.0.

In Release v95.0.1, V100.0, v105.0, v107.0, the issue does NOT reproduce, but the new link is being opened in a NEW WINDOW, even if the "open links in tabs instead of new windows" is activated.

The investigation of this issue had its ups and downs, firstly because the issue can not be reproduced with mozregression builds and also because I initially found a range that apparently was correct, only later to discover that the profiles might get corrupted, causing this issue to reproduce on said profiles. The whole time, the issue felt intermittent, causing me to suspect that the profile manager would need to be disabled in order for it to reproduce.

The steps to reproduce look like this:

1. Install a browser.
2. open it
3. disable updates
4. activate session restore
5. set as default browser
6. open a few tabs
7. close the session
8. open thunderbird
9. click a web link

Actual: The Previous session is restored, but the link that was clicked does not load unless clicked again.

Expected: The previous session is loaded back, along with the newly clicked link.

The behavior of the older browser versions is as follows: The previous session is restored and the newly clicked link is opened in a new window (even if the "open links in new tabs instead of new windows" is checked), this being a secondary issue observed in the "unaffected" builds. It might be relevant to how this issue was regressed.

In conclusion: Considering the suspicion of intermittent reproduction and that the profiles are getting corrupted, I've opened unaffected builds, with the same profile, incrementally one after the other, until the issue started reproducing. Coincidently, the issue starts reproducing in the first build of December 2022, v109.0a1 (2022-12-01) 20221201161829. If then, an unaffected build is opened with this profile, it will reproduce.

First bad: v109.0a1 (2022-12-01) 20221201161829
Last good: v109.0a1 (2022-11-30) 20221130214707

Furthermore, this issue could not be reproduced on other OSes, so I am assuming this is MacOS specific.

Comment 4

[james]

I submitted a duplicate (bug 1825965) because I didn't see this bug when I searched for existing issues.

I tested by going to about:profiles and creating a new profile to test with. This bug only occurs if "Open previous windows and tabs" is enabled in the browser settings. Otherwise it behaves properly. In the comments to bug 1825965, I posted a video of the bug occurring. I had to make the video short and blurry to be under the 10 MB upload limit.

As noted by :danibodea above, prior versions of Firefox would open the "new tab" as a new window hidden behind the previous session's window. That behavior occurred on macOS, but not on Windows 10. On Windows 10, a new tab would correctly open as a new tab.

I am running Firefox 111.0.1 on macOS Monterey 12.6.4. (My old Windows computer has died, so I can no longer compare the behavior on macOS to that on Windows.)

Comment 6

[Stephen A Pohl [:spohl]]

(In reply to Daniel Bodea [:danibodea] from comment #2)

First bad: v109.0a1 (2022-12-01) 20221201161829
Last good: v109.0a1 (2022-11-30) 20221130214707

This seems to point to a regression range of https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e4ac0dccafdddfa13e91dec0e852d001ca7c528e&tochange=9b463a0b6bfa9b2bbcd4c12590934977cc90aa9c, but I can't immediately see what changeset might have caused this change in behavior.

Comment 8

[james]

I have reluctantly changed my macOS default browser from Firefox to Chrome, because this bug prevents Firefox from doing the basics of what a system default browser is supposed to do (i.e. opening links from third-party applications).

By the way, this bug doesn't just occur when clicking links in other applications. It also occurs when double-clicking HTML files in the Finder.

Firefox's ability to act as the system default browser on macOS is fundamentally broken.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Shot in the dark, but if I expand the regression range a bit, I see that bug 1538028 and bug 1799692 also landed around the time this regressed. Is it possible they're contributing?
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e46a721b2af4&tochange=9b463a0b6bfa9b2bbcd4c12590934977cc90aa9c

Comment 10

[Nika Layzell [:nika] (ni? for response)]

TL;DR: The OS opens the browser, then sends a second command-line after startup. The second command line arrives before the first window is opened. There are 2 issues preventing the tabs from opening: pending window open notifications are being resolved too early, and SessionStore replaces all tabs in the first window if the startup command-line has no specified URLs.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)

Shot in the dark, but if I expand the regression range a bit, I see that bug 1538028 and bug 1799692 also landed around the time this regressed. Is it possible they're contributing?
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e46a721b2af4&tochange=9b463a0b6bfa9b2bbcd4c12590934977cc90aa9c

That seems unlikely to me. The checks added by those interfaces generally lead to crashes, so wouldn't be leading to an issue like this. The setup for handling open calls from other applications on macOS is definitely a bit weird & different from other browsers, but it seems unlikely that those bugs would change things here.

I have been looking into this issue for other reasons though (specifically due to bug 1845407), and I think I've found the root of the issue. When Firefox is being opened to load a URL (such as through open https://example.com) we are not processing the OpenURL commands during the command line building phase hack which was added in bug 531552 (we may want to remove this hack anyway due to bug 1845407, which would make this path be taken all of the time). Instead, the commands are being handled as separate command lines immediately after startup by the frontend code in nsBrowserContentHandler.

Until bug 1792466, a second command line this early would lead to a new window being opened to load the passed URL, rather than it opening in a new tab within the current window. That patch changed the logic such that we now track browser windows as they are opening, rather than only once they have opened, meaning that the new tab should open in the newly created pop-up window.

Unfortunately, while this did effectively fix bug 1600153, it ended up causing this issue for 2 major reasons:

First, the pending window tracker was unfortunately broken about a month ago in bug 713713, where a check was added to resolve the pending window open call early if an "unload" event is fired (https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/browser/modules/BrowserWindowTracker.sys.mjs#245-257). This check will fire even for normal and successful loads when the initial about:blank page in the newly-opened window is unloaded, meaning that the pending window promise will be resolved too early. In the case of this bug, it appears this manifests as https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/browser/components/BrowserContentHandler.sys.mjs#950 failing due to browserDOMWindow being null, as the window had not finished loading yet before we tried to load a URL into it.

We probably want to fix this by removing that "unload" listener and using something more reliable to detect that the window was actually destroyed, such as perhaps listening to a browsing-context-discarded observer notification (or similar) instead.

As is demonstrated by the age of this bug, though, bug 713713 didn't cause all of the issues here.

Secondly, there is a poor interaction between bug 1792466's changes and SessionStore, which means that if your browser is configured to restore the previous session, these new tab loads would be clobbered. SessionStore restores the state from the previous session into the first browser window automatically after both browser-delayed-startup-finished and the session state file has been read from disk, meaning that it will likely happen after we have added our new tabs due to the loadURL command from the OS.

Normally, when SessionStore is restoring a window, it will close all tabs which have already been opened in the browser window and replace them with the tabs from the session being restored. This avoids a brand new blank tab being added to the end of the tabs list in each newly-restored window, as by default when the window is opened it will have a single tab. This behaviour is special-cased to be disabled on the first browser window during startup if an explicit URL was passed from the command line (https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/browser/components/sessionstore/SessionStore.sys.mjs#1938-1939, https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/browser/components/sessionstore/SessionStore.sys.mjs#5962-5982), such that the URL which the browser is opened with will not be clobbered (as it will have replaced the initial blank tab due to having been specified on the command line).

Unfortunately, in the macOS case, the initial command line will have been empty, so SessionStore will be configured to replace all existing tabs in the window. This then leads to the newly loaded tabs from the early-startup secondary command lines being closed, and replaced with the previous session.

Fixing this will be more difficult, and I'm probably not the right person to be proposing a specific approach (this is all in frontend code), but there are various approaches we could take. My impression is that the current handling (where e.g. the SessionStore component inspects window.arguments to see if it matches the default value from nsBrowserContentHandler) is a bit fragile, and leads to assumptions being made about things like timing. It is possible on all platforms, not just macOS (though it is much more common on macOS) to have remote command lines delivered to the BCH very early during startup, which would also lead to this behaviour. One possibility, though it may be complex to actually implement, is to delay the decision about what tabs will be loaded into the first browser window until after SessionStore has had a chance to restore the previous session, and then perform the loading of whatever new content should be loaded on top of that, depending on whether or not a previous session was restored. With an approach like that, we could then have future command lines which occur before the session has been restored add the URLs to load to a queue which isn't checked until the session has been restored, rather than finding a window and loading into it directly.

Finally, we should also probably look into adding new tests for this behaviour to ensure that it doesn't regress in the future. My local testing and reproduction of the failure on a test build of firefox was done by doing the following:

open -a /path/to/obj-firefox/dist/Nightly.app --stderr output_file -o output_file https://example.edu

You can pass explicit arguments to the application as well with --args ... (like we do for ./mach run --macos-open), though it's important to "open" the URL rather than pass it as a command line argument to reproduce the issue. It might be possible for us to add a marionette test or similar which invokes the binary in this way, and makes sure that the browser loads as expected.

Comment 11

[:Gijs (he/him)]

(In reply to Nika Layzell [:nika] (ni? for response) from comment #10)

TL;DR: The OS opens the browser, then sends a second command-line after startup. The second command line arrives before the first window is opened. There are 2 issues preventing the tabs from opening: pending window open notifications are being resolved too early, and SessionStore replaces all tabs in the first window if the startup command-line has no specified URLs.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)

Shot in the dark, but if I expand the regression range a bit, I see that bug 1538028 and bug 1799692 also landed around the time this regressed. Is it possible they're contributing?
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e46a721b2af4&tochange=9b463a0b6bfa9b2bbcd4c12590934977cc90aa9c

That seems unlikely to me. The checks added by those interfaces generally lead to crashes, so wouldn't be leading to an issue like this. The setup for handling open calls from other applications on macOS is definitely a bit weird & different from other browsers, but it seems unlikely that those bugs would change things here.

I have been looking into this issue for other reasons though (specifically due to bug 1845407), and I think I've found the root of the issue. When Firefox is being opened to load a URL (such as through open https://example.com) we are not processing the OpenURL commands during the command line building phase hack which was added in bug 531552 (we may want to remove this hack anyway due to bug 1845407, which would make this path be taken all of the time). Instead, the commands are being handled as separate command lines immediately after startup by the frontend code in nsBrowserContentHandler.

Thanks for sharing your investigations here!

Do we actually know why there are 2 command lines here instead of just 1? That is, to avoid some of the issues here and in bug 1600153, we'd need to know at startup whether we have got or will get the second command line. When I looked into this in the past, I didn't really make any headway.
ISTM that if we can't figure this out, we'll always end up with some variation of bug 1600153, because we cannot make opening Firefox without a URL wait indefinitely for a URL to be provided on the commandline, which may never arrive... or does the second commandline always arrive, sometimes with, sometimes without a URL?

Comment 12

[Nika Layzell [:nika] (ni? for response)]

(In reply to :Gijs (he/him) from comment #11)

Thanks for sharing your investigations here!

Do we actually know why there are 2 command lines here instead of just 1? That is, to avoid some of the issues here and in bug 1600153, we'd need to know at startup whether we have got or will get the second command line. When I looked into this in the past, I didn't really make any headway.

The Cocoa SDK delivers files which are to be opened using a Cocoa API rather than using the SDK. The second command line is actually simulated by the browser in response to that event coming from the OS: https://searchfox.org/mozilla-central/rev/d81e60336d9f498ad3985491dc17c2b77969ade4/toolkit/xre/MacApplicationDelegate.mm#399-403.

On more recent versions of macOS (including our minimum supported version in Nightly), we can use the application:openURLs: callback on the delegate, though we've historically used other approaches as that API is only supported since macOS 1.13.

ISTM that if we can't figure this out, we'll always end up with some variation of bug 1600153, because we cannot make opening Firefox without a URL wait indefinitely for a URL to be provided on the commandline, which may never arrive... or does the second commandline always arrive, sometimes with, sometimes without a URL?

As far as I am aware, it is possible for us to add a handler for applicationDidFinishLaunching:, which should be fired after the openURLs commands which caused the browser to start have been delivered, so it might be possible for us to always emit some type of callback into JS code after we know that all start-up command lines have been processed, or potentially even delay processing the start-up command line until this callback is notified. The delay approach is roughly the one taken in https://phabricator.services.mozilla.com/D184524, though I'm a bit concerned about starting the full app early, and the consequences of exiting and re-entering [NSApp run], so we might want to avoid doing that and instead handle it async.

Comment 13

[emikaadeo]

Hi,

with Firefox 117.0 release we're back to the situation where:
Firefox starts with the last session but the clicked link does not open at all.

in 116.0.3 was:
Firefox starts with the last session and the clicked link opens in a new tab. That's not perfect but at least the website opened.

Comment 14

[Sören Hentzschel]

@emikaadeo: There was never a fix for this issue so I doubt that it worked in Firefox 116.0.3 (and in fact it never worked for me since the issue exists). What you describe (assuming that you mean that the website opened in a new window, as in a new tab would be the expected behaviour) was the behaviour until Firefox 108.

Comment 15

[emikaadeo]

@Sören Hentzschel
Let me explain how it works for me on macOS Ventura with 116.0.x/115.0.x
Simple Firefox settings:

• browser always starts in private mode
• browser starts with default Firefox page
  When i click on a link in external app, Firefox starts with ONE window and TWO tabs. First tab is empty and second is a website from a clicked link.
  That's not perfect but at least the website opened.

On 117.0 Firefox opens ONE window with empty tab and nothing else.

Comment 16

[Alexandru Trif, Desktop Test Engineering [:atrif]]

(In reply to emikaadeo from comment #15)

@Sören Hentzschel
Let me explain how it works for me on macOS Ventura with 116.0.x/115.0.x
Simple Firefox settings:

• browser always starts in private mode
• browser starts with default Firefox page
  When i click on a link in external app, Firefox starts with ONE window and TWO tabs. First tab is empty and second is a website from a clicked link.
  That's not perfect but at least the website opened.

On 117.0 Firefox opens ONE window with empty tab and nothing else.

Hello! Maybe you are now hitting bug 1850828 / bug 1850848 on 117.

Comment 19

[steve]

Can no longer reproduce the problem with macOS 13.6.4, Firefox 123.0. Also tested macOS 12.7.3, Firefox 123.0.
I do get the described expected result: "Firefox starts with the last session and the clicked link opens in a new tab."

Think we can call this fixed or worksforme since there is no specific commit resolving this problem. Do you agree Sören?

Comment 20

[james]

Can no longer reproduce the problem with macOS 13.6.4, Firefox 123.0. Also tested macOS 12.7.3, Firefox 123.0.

It is currently working for me on macOS 12.7.3 and Firefox 123.0. I just tested quickly under my current user profile. I did not try creating a new profile to test it.

Think we can call this fixed or worksforme since there is no specific commit resolving this problem.

Before marking it as fixed, it should be thoroughly tested. Also, the specific code change that fixed the problem should be identified and documented. Finally, a new regression test should be added, as @nika suggested, so the problem doesn't reappear:

Finally, we should also probably look into adding new tests for this behaviour to ensure that it doesn't regress in the future.

Comment 21

[james]

Before marking it as fixed, it should be thoroughly tested. Also, the specific code change that fixed the problem should be identified and documented. Finally, a new regression test should be added, as @nika suggested, so the problem doesn't reappear

On second thought, the regression test should be written first. That will make it quicker and easier to thoroughly test on all target platforms to confirm that the bug is actually fixed. It will also make it quicker to identify which code change fixed the bug (assuming it's actually fixed).

Comment 23

[Stephen A Pohl [:spohl]]

As Nika pointed out in comment 12, the logic involved here was reworked in bug 1845407 and subsequent bugs.