Open Bug 1718104 Opened 4 years ago Updated 3 months ago

A link with custom URI scheme and target="_blank" opens a new empty tab and does not close it

Categories

(Core :: DOM: Navigation, defect, P3)

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

People

(Reporter: saschanaz, Unassigned)

References

(Blocks 1 open bug)

Details

The example is Windows-only but any working custom URI scheme should apply.

data:text/html,<a href="ms-windows-store://pdp/?PFN=Microsoft.MicrosoftSolitaireCollection_8wekyb3d8bbwe" target="_blank">click me</a>

  1. Copy-paste the above to the url bar to open it
  2. Click the link

Expected: No new tab should be there after it opens the corresponding app (Microsoft Store in this case). Chrome opens one but immediately close it after closing the app opener dialog.
Actual: A new about:blank tab is kept open with no use.

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core

(Move to DOM: Navigation, but I could be wrong, feel free to correct me)

Component: DOM: Core & HTML → DOM: Navigation

needinfo'ing Nika because she says this bug is a duplicate of another bug.

Severity: -- → S4
Flags: needinfo?(nika)
Priority: -- → P3

(In reply to Chris Peterson [:cpeterson] from comment #3)

needinfo'ing Nika because she says this bug is a duplicate of another bug.

And she offered to find that bug number.

I'm not sure it's actually a duplicate of that bug (bug 1678965), but I think they should be at least linked together.

ni? :gijs, as you've worked on this sort of issue quite a bit lately.

Flags: needinfo?(nika) → needinfo?(gijskruitbosch+bugs)
See Also: → 1678965

bug 1678965 is about downloads, and this is about external protocol handling. In this case, I'm not sure if this has ever worked or how strongly it's been considered? The issue here is that the DOM code that opens the link doesn't know if we're going to need to open this in a web handler. We could decide to always use a new, disconnected browsingcontext for (_blank or even all!) external protocol navigations but that has some web consequences and I don't know where we're at with that - Anne or Johann would be better bets on that one.

If we assume we need to continue to support a website interacting with the result of the scheme opening, then the point at which we could close the browsing context comes after we ask the handler service what to do (and that potentially asks the user what to do, in the opened tab, if no default is set for the scheme). I think that like bug 1678965, we'd need a way for the external protocol code / handler service, to ping the DOM code and go "hey, I don't need this browsing context anymore if you opened it just for this link" - because while the DOM code didn't know whether we needed the context to open the link, the external protocol handler code is oblivious to whether or not the context was only opened for the purposes of opening the link, or needs to stay alive.

Nika, does that sound like an accurate summary?

Johann, do we have a dupe for this and/or do you know what the state of play is for how we deal with external protocol navigations that open _blank things (ie whether we need to guarantee something usable as the return value of window.open("web+myscheme:blah", "_blank"))?

We'll be looking to improve the external handler stuff as part of MR1.1 so we could investigate taking up this issue as part of that...

Flags: needinfo?(nika)
Flags: needinfo?(jhofmann)
Flags: needinfo?(gijskruitbosch+bugs)

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

bug 1678965 is about downloads, and this is about external protocol handling.

I think we'd want to have it with roughly the same behaviour as doing a download, where once the load an external program, we'd remove the tab in roughly the same way and situations. I think I had assumed that it would use a similar codepath, because I'd mixed up nsExternalProtocolHandler and nsExternalAppHandler in my head :-P

The issue here is that the DOM code that opens the link doesn't know if we're going to need to open this in a web handler. We could decide to always use a new, disconnected browsingcontext for (_blank or even all!) external protocol navigations but that has some web consequences and I don't know where we're at with that - Anne or Johann would be better bets on that one.

I don't think that's the issue here, is it? The issue is moreso we're opening a new blank page and not closing it after the load actually doesn't load any content (like the situation with downloads). I don't think we'd want to change anything by making protocol loads open in new windows automatically.

If we assume we need to continue to support a website interacting with the result of the scheme opening, then the point at which we could close the browsing context comes after we ask the handler service what to do (and that potentially asks the user what to do, in the opened tab, if no default is set for the scheme). I think that like bug 1678965, we'd need a way for the external protocol code / handler service, to ping the DOM code and go "hey, I don't need this browsing context anymore if you opened it just for this link" - because while the DOM code didn't know whether we needed the context to open the link, the external protocol handler code is oblivious to whether or not the context was only opened for the purposes of opening the link, or needs to stay alive.

We can probably take advantage of the same "docshell.newWindowTarget" attribute as we use for downloads if we add a nsHashPropertyBag base to nsExtProtocolChannel to capture it. It should be set on the channel when we open it for document navigations from the DocumentLoadListener. With that we can do something similar to what the nsExternalAppHandler does with MaybeCloseWindowHandler here: https://searchfox.org/mozilla-central/rev/77f0b36028b2368e342c982ea47609040b399d89/uriloader/exthandler/nsExternalHelperAppService.cpp#1724-1736. That would get us the relevant info all of the way to nsExtProtocolChannel::OpenURL, and we could combine it with checking for a TYPE_DOCUMENT load, and close the window when needed from handler service.

Johann, do we have a dupe for this and/or do you know what the state of play is for how we deal with external protocol navigations that open _blank things (ie whether we need to guarantee something usable as the return value of window.open("web+myscheme:blah", "_blank"))?

We do presumably need to continue to guarantee that, but it would be OK for us to close the window when the load actually happens like we do for downloads.

Flags: needinfo?(nika) → needinfo?(gijskruitbosch+bugs)

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

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

bug 1678965 is about downloads, and this is about external protocol handling.

I think we'd want to have it with roughly the same behaviour as doing a download, where once the load an external program, we'd remove the tab in roughly the same way and situations. I think I had assumed that it would use a similar codepath, because I'd mixed up nsExternalProtocolHandler and nsExternalAppHandler in my head :-P

Yeah, I agree we want the same behaviour - but I don't think much of the implementation can be shared, though I'm not clear on whether you agree and/or whether I'm missing something - see below. :-)

The issue here is that the DOM code that opens the link doesn't know if we're going to need to open this in a web handler. We could decide to always use a new, disconnected browsingcontext for (_blank or even all!) external protocol navigations but that has some web consequences and I don't know where we're at with that - Anne or Johann would be better bets on that one.

I don't think that's the issue here, is it? The issue is moreso we're opening a new blank page and not closing it after the load actually doesn't load any content (like the situation with downloads). I don't think we'd want to change anything by making protocol loads open in new windows automatically.

Well, my point was that if we enforced blank disconnected browsing contexts for these loads, then the external protocol handler could always close/destroy the context once it decided no web handler was going to be used, as it would know for sure that that was safe. As it is, it needs to cooperate with DOM code to do that (either get the info from DOM code and do it itself, or tell DOM code "I don't need this, if you also don't need it you should throw it away."). IOW, it would simplify this a little, I think (though that by itself is obviously not a good enough reason to make web standards changes :-) .

If we assume we need to continue to support a website interacting with the result of the scheme opening, then the point at which we could close the browsing context comes after we ask the handler service what to do (and that potentially asks the user what to do, in the opened tab, if no default is set for the scheme). I think that like bug 1678965, we'd need a way for the external protocol code / handler service, to ping the DOM code and go "hey, I don't need this browsing context anymore if you opened it just for this link" - because while the DOM code didn't know whether we needed the context to open the link, the external protocol handler code is oblivious to whether or not the context was only opened for the purposes of opening the link, or needs to stay alive.

We can probably take advantage of the same "docshell.newWindowTarget" attribute as we use for downloads if we add a nsHashPropertyBag base to nsExtProtocolChannel to capture it. It should be set on the channel when we open it for document navigations from the DocumentLoadListener. With that we can do something similar to what the nsExternalAppHandler does with MaybeCloseWindowHandler here: https://searchfox.org/mozilla-central/rev/77f0b36028b2368e342c982ea47609040b399d89/uriloader/exthandler/nsExternalHelperAppService.cpp#1724-1736. That would get us the relevant info all of the way to nsExtProtocolChannel::OpenURL, and we could combine it with checking for a TYPE_DOCUMENT load, and close the window when needed from handler service.

This wfm, but it's different from the approach in bug 1678965 right?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(nika)

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

This wfm, but it's different from the approach in bug 1678965 right?

IIRC the issue in the case of bug 1678965 is that the code in MaybeCloseWindowHandler can't close the tab because to close the tab for a download, we need to know which window opened the download tab to re-parent the download dialog to that window. As we created the new tab in that specific case from chrome JS we aren't tracking the CrossGroupOpener like we would for a content-triggered opener, so we don't know which window to re-parent the dialog to and need to keep the window open.

The core problem in this situation is that we never try to close a window if it was opened just to load an external protocol, whether or not we are tracking the opener window and whether or not the "docshell.newWindowTarget" attribute (which corresponds to nsIWebNavigation::LOAD_FLAGS_FIRST_LOAD) is set. If we start checking that flag and do dialog reparenting like we do in the nsExternalAppHandler (using MaybeCloseWindowHelper) we'll handle all of the cases which we're currently handling for downloads with external protocols as well, and when we fix bug 1678965, we'll fix that specific situation with external protocols as well.

Flags: needinfo?(nika)

It probably is a good idea to re-use the close-empty-tab logic that we use for downloads here as a fallback, but most of the time we should know that a custom protocol handler isn't going to load an actual page much earlier than we know that for a download, so it would be nice if we could just avoid opening a new tab to begin with in those cases...

I don't know the answer to comment 6, CC'ing Paul for awareness.

Flags: needinfo?(mail)

I've come across this rather annoying error. And even more so when it coincides with this other bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1966584

The result is that with Firefox the user is "hung". With Edge neither of these two errors occurs.

Demo video: https://youtu.be/cFeUVSQSKZc

See Also: → 1966584

Marking as an enterprise important bug because custom protocols are used a lot in enterprise.

Blocks: fx-enterprise
You need to log in before you can comment on or make changes to this bug.