mozilla / multi-account-containers

Firefox Multi-Account Containers lets you keep parts of your online life separated into color-coded tabs that preserve your privacy. Cookies are separated by container, allowing you to use the web with multiple identities or accounts simultaneously.
https://addons.mozilla.org/firefox/addon/multi-account-containers/
Mozilla Public License 2.0
2.73k stars 342 forks source link

window.open in greasemonkey script crashes tabs #1771

Open Narcha opened 4 years ago

Narcha commented 4 years ago
// test this on an empty HTML file
var a = document.createElement("button");
a.innerHTML = "click me"
a.onclick = ()=>{
  window.open("https://example.com");
}

document.getElementsByTagName("body")[0].appendChild(a)

Calling "window.open()" in a Greasemonkey script in a non-default tab container crashes the tab. It does not crash

Actual behavior

Tab crashes when window.open() is called from a Greasemonkey script.

Expected behavior

The tab not crashing.

Steps to reproduce

  1. create a new HTML file, add body tags
  2. install the given Greasemonkey script, configure it to affect the page
  3. click the generated button

Notes

I found #1549 , but I believe this is a different bug. Still, I am not quite sure if this is a bug with containers or with Greasemonkey.

hckr commented 4 years ago

Still, I am not quite sure if this is a bug with containers or with Greasemonkey

The same issue occurs in normal add-ons, see the issue referenced above.

hckr commented 4 years ago

I found a workaround: using window.wrappedJSObject instead of plain window. It seems to work also in Greasemonkey userscripts.

For example:

window.wrappedJSObject.open("https://mozilla.org");

I'm not necessarily fan of this solution/hack though.

Narcha commented 4 years ago

@hckr thanks for the workaround, it works great.

hckr commented 4 years ago

Unfortunately, I discovered that this workaround causes problems with messaging between windows via postMessage – responding with e.source.postMessage from popup window to the main window (e.source) fails.

So, I guess it solves most issues, but not all.

(It isn't connected to this extension in any way, rather to how window.wrappedJSObject is implemented, I guess.)

filbo commented 4 years ago

It is very likely you would need to use XPCNativeWrapper or something, to re-wrap, at some point in the whole procedure. I'm only speaking from docs, haven't tried any of this myself. https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts mentioned XPCNativeWrapper(), but the document it points to for 'much more detail on this', Xray vision, doesn't mention that at all, so who knows. Perhaps you would need Components.utils.waiveXrays(), which is mentioned in that doc.

And by 'at some point in the whole procedure' I mean, I don't have a firm enough grounding to know if you would need to do that in the popup window, main window, both, or what...

And, of course, if you're trying to write an extension which is portable across FF, older FF, Chrome. etc., you're going to be up to your eyeballs in polyfills, browser sniffing, or worse.

hckr commented 4 years ago

@filbo I already use something specific to Firefox. What I need is quite simple – sens message to a window created with window.wrappedJSObject.open, and then in the receiving handler send a response (with is normally possible via e.source.postMessage where e is an event received by onmessage handler). This handler is also created by the add-on, both windows have the same domain loaded inside. It works without wrappedJSObject stuff.

filbo commented 4 years ago

Explore. Does e.wrappedJSObject exist? Then maybe you need to use e.wrappedJSObject.source.postMessage(). Or if it doesn't exist, maybe you need to wrap it with XPCNativeWrapper(). Or maybe this stuff Just Won't Work. The people who wrote / maintain the containers stuff should have a much better mental model of this than someone who's just looking at docs like me...

hckr commented 4 years ago

I explored it a bit and came to a conclusion it won't work. .wrappedJSObject does exist on this event, but messages aren't received by the other end.

I wrote all this just in case someone came by and happened to know the solution.