Gijs, another fine bug for your consideration 🍻

I have been looking into this. And I am wondering if we could nsIOpenWindowInfo for this. It looks like it's already passed in from a lot of callers (at least in C++). I think for openLinkIn we would need some way of creating the interface from JS. Afterwards we could just add hasValidTransientUserGestureActivation to the interface in bug 1738694.

(In reply to Tom Schuster (MoCo) from comment #11)

I have been looking into this. And I am wondering if we could nsIOpenWindowInfo for this. It looks like it's already passed in from a lot of callers (at least in C++). I think for openLinkIn we would need some way of creating the interface from JS.

Yes.

Afterwards we could just add hasValidTransientUserGestureActivation to the interface in bug 1738694.

I think this would potentially work, but I think there might be some places right now that assume that only C++ sends these objects and make decisions as a result (cf. https://searchfox.org/mozilla-central/rev/b72e9d7d63bf499d1d8168291b93d4ec7fde236e/browser/base/content/tabbrowser.js#2836-2839 ). So we'd have to be a little careful (maybe audit use of this prop in frontend code right now :-( )

But I think an ideal end-state here would be that we get rid of the magic-index-into array-based window.arguments handling.

To do that, I think we'd need a way to include also stuff that's currently separate, like the actual URL to load, into the object. Does that seem feasible?

The other thing I'm wondering is if we'd then pass an array of these objects for cases where we want to open more than one URL... Nika/Tom, wdyt?

To do that, I think we'd need a way to include also stuff that's currently separate, like the actual URL to load, into the object. Does that seem feasible?

I think that is probably something Nika needs to decide. It would probably be confusing to include the URL into that object, because we already pass the URL as an argument in C++ separately to functions using nsIOpenWindowInfo.

Passing an array of info objects should be possible in theory.

In general nsIOpenWindowInfo is designed with the goals of the backend in mind, as it tries to effectively act as a way to pass an object into the frontend, which will create a new tab or window for us, and then get some action to happen in C++ when the <browser> element which the info is attached to is created (either attaching an existing BrowserParent for a content-process window.open, or calling the BrowsingContextReadyCallback). It's not been designed around the idea of actually starting a navigation from the frontend using it, code which does that does so by starting the navigation from the nsIBrowsingContextReadyCallback. It's also generally designed for opening content windows, rather than for opening chrome windows, as we also use it when creating a window in a new tab.

My general expectation is that that isn't exactly what frontend wants for arguments to pop-up windows with window.arguments, and I worry a bit that combining both sets of requirements together, we'll make something more confusing. I expect that frontend probably has other flags which aren't related to the content document being loaded in the new window which they want to pass (e.g. window state for the browser window, or you may want to just open a new window with default URLs?), and probably handles navigations differently (IIRC because of how you're called, you generally have the newly opened window start the navigation, rather than starting it from something like a callback).

Perhaps an approach might be to change consumers of window.arguments to instead always pass a single argument which is an object with the properties frontend wants to depend on? We could perhaps change how the backend works such that window.arguments (and the "arguments" argument to nsIWindowWatcher::openWindow) is just the object directly in that case (perhaps requiring some signaling at first), rather than an array-like object, and eventually rip out the nsIArray stuff? Not sure whether that would fix the issues you're looking to fix or not.

As an intermediate solution, I have been experimenting with adding a nsIPropertyBag as the new last argument. We could then add existing or new arguments to the bag instead and slowly move to something else.

I bug 1738694 I now added the nsIPropertyBag parameter that I suggested. We could now start moving parameters from the call into this object.