1560842 - Calling "window.open()" in TestRunnerActivity's parent process fails with "No chrome package registered for chrome://browser/content/browser.xul"

Status: NEW

(GeckoView :: General, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

For several test suites like web-platform-tests the org.mozilla.geckoview.test/org.mozilla.geckoview.test.TestRunnerActivity application is used on Android. While working on bug 1559120 I noticed that calling window.open() in the parent process doesn't create a new window but fails with:

No chrome package registered for chrome://browser/content/browser.xul

So why are we trying to use browser.xul instead of geckoview.xul which is used for the initial window and is located here:

chrome://geckoview/content/geckoview.xul

If we don't allow opening a new window from the parent process, which we actually might want, we should raise an exception instead.

Nick, any thoughts on that?

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Is window a chrome window in this case? If so, I don't think that makes much sense for GV. If we're talking about content loaded in the parent then that's a different story and it should work.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hm, I simply opened the dev tools and connected to the parent process. Calling window.open() there I get this failure, and it is a chrome window yes. Not sure what you are referring to with content loaded. Isn't that only happening in the child processes?

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #2)

Hm, I simply opened the dev tools and connected to the parent process. Calling window.open() there I get this failure, and it is a chrome window yes.

Yeah, I wouldn't expect that to work, as it doesn't really make sense for a GV app. I guess if Marionette or w-p-t needs it to work we may be able to figure something out, though.

Not sure what you are referring to with content loaded. Isn't that only happening in the child processes?

You could've connected to a tab that was loaded in the parent process, in which case I'd expect window.open() to work.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #3)

Yeah, I wouldn't expect that to work, as it doesn't really make sense for a GV app.

Ok, so the question is if the failure should be silently ignored or raised as an exception.

I guess if Marionette or w-p-t needs it to work we may be able to figure something out, though.

To make that work in Marionette I pass the Marionette's new window command to its framescript, and call window.open() from there. That actually works fine for the time being.

Comment 5

[Chris Peterson [:cpeterson]]

James says we would need to add a special window.open() delegate for Marionette to make this work. Perhaps that Marionette delegate could just be implemented in TestRunnerActivity and not GV itself?

I will needinfo Henrik after he returns from PTO on July 19.

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] (away July 6th - July 19th) from comment #4)

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #3)

Yeah, I wouldn't expect that to work, as it doesn't really make sense for a GV app.

Ok, so the question is if the failure should be silently ignored or raised as an exception.

I think it should just raise an exception. There are tons of things that mean something specific in a content JS context that "kinda-sorta" work in a chrome JS context; it's not worth trying to determine them all.

I guess if Marionette or w-p-t needs it to work we may be able to figure something out, though.

To make that work in Marionette I pass the Marionette's new window command to its framescript, and call window.open() from there. That actually works fine for the time being.

If my mental model is correct, that makes sense.

Comment 7

[Chris Peterson [:cpeterson]]

To make that work in Marionette I pass the Marionette's new window command to its framescript, and call window.open() from there. That actually works fine for the time being.

Henrik, is your Marionette window workaround adequate?

snorp says we would need to add a special window.open() delegate for Marionette to fix this work correctly. Perhaps that Marionette delegate could just be implemented in TestRunnerActivity and not GV itself.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

For Marionette it is fine when running the code in a framescript as stated above in comment 4.

The remaining issue for this bug is outside of the Marionette scope.

Comment 9

[Chris Peterson [:cpeterson]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #8)

The remaining issue for this bug is outside of the Marionette scope.

Henrik, what is the remaining issue? That calling window.open() from remote dev tools fails to open a new tab on the device?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yes, basically the steps which I have given in comment 2:

I simply opened the dev tools and connected to the parent process. Calling window.open() there I get this failure.