1506782 - Add support for opening and closing of tabs for GeckoView applications (Marionette and WebDriver BiDi)

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P1)

Description

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

On bug 1506643 I set the `browser.newtabpage.enabled` pref to false to prevent loading the `about:newtab` page when opening new tabs. I would like to see something similar for Fennec, which also shows such a page, but the preference from above doesn't seem to be related.

James, mind telling me how this page can be disabled? Thanks.

Comment 1

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

I don't really have knowledge of how this works in the Fennec UI. Someone on Susheel's team should be able to help.

Comment 2

[:sdaswani]

As a P3 this will show up in triage at some point.

Comment 3

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

(In reply to :sdaswani only needinfo from comment #2)
> As a P3 this will show up in triage at some point.

No, because this bug is not part of the Fennec product but Marionette. As such it won't appear in any triage you do.

Comment 4

[:sdaswani]

Sorry - can you help me with priority here? What does this work block, and how important is it?

Comment 5

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

With this work I want to make navigation for Marionette tests more reliable, and faster. The tests don't need the `about:newtab` experience including activity stream, but only have to open `about:blank`.

The only information needed here is how to turn off activity stream so that just `about:blank` gets loaded when opening new tabs.

Comment 6

[:sdaswani]

Liz how can we prioritize this against our other Fennec needs?

Comment 7

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Probably pretty easy to do, and it's good to improve automated test reliability - why not make it a P2. Vlad can your team take a look?

Comment 8

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

I actually don't understand why we make it such a complicated process from just a question about which preference to use to display `about:blank` and not the activity stream in Fennec. Is there no such preference yet?

Again, this is not a bug for Fennec but Marionette and as such any whiteboard/severity setting doesn't apply.

Comment 9

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

So I found that through the setting I can set the home page to "about:blank", and to let Fennec use it for each new tab. But I have problems figuring out how Marionette could set it, because those don't seem to be preferences.

Both are android:key entries here:

https://searchfox.org/mozilla-central/rev/227f5329f75bd8b16c6b146a7414598a420260cb/mobile/android/app/src/main/res/xml/preferences_home.xml#14,20

Where are those preferences set? James, could you please help? Thanks

Comment 10

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

We weren't able to get this done via bug 1533058, because of the following call:

tab = this.tabBrowser.addTab(null);

Usually it would allow to pass in about:blank as URL, but as jgraham said null was necessary to use for GeckoView to kill the race condition. I haven't further investigated yet, but I hope that with being able to set the above preferences that we won't need neither of those.

Comment 11

[Nick Alexander :nalexander [he/him]]

Parachuting into this conversation. In general there's a big split between App-level settings and Gecko-level settings, and in general Mozilla's Apps (Fennec, Fenix, r-b, etc) don't have a uniform way to manage App-level settings in automation. I have considered writing harness code to set Android SharedPreferences (which is what :whimboo is ultimately referring to in #c9), but it's very fiddly and requires either support from within Marionette/Gecko or the App or to have android:debuggable="true".

I have had success adding per-App Intent arguments to control these types of things from automation: e.g., https://github.com/mozilla-mobile/fenix/issues/1525.

For this particular issue, in my browsertime harness, I use the "skipstartpane" extra for Fennec and launch with a URL before driving Marionette: see https://searchfox.org/mozilla-central/search?q=EXTRA_SKIP_STARTPANE&redirect=false for the Intent argument.

Comment 12

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

Yeah, comment #11 captures the problem -- an intent argument is a good way to add this kind of thing to Fennec.

Comment 13

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

This bug should also cover GeckoView given that Fennec goes away soon. So I assume that an intent argument will also work with GeckoView based browsers?

Comment 14

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

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

This bug should also cover GeckoView given that Fennec goes away soon. So I assume that an intent argument will also work with GeckoView based browsers?

Each product would need special attention, but yes, it's possible -- GV doesn't have any role in this.

Comment 15

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

I would like to get back to this bug again. How is it possible these days to set about:blank for a new tab that gets e.g manually opened via the tabs pane / view? The intent arguments -a android.intent.action.VIEW -d about:blank only seem to affect the initial start page but not any tab that gets opened later. Is that a GeckoView specific setting or related to the Android app embedding GeckoView?

Note that we won't add support for that in Fennec anymore.

Nick maybe you can help or forward the question appropriately? Thanks.

Comment 16

[Nick Alexander :nalexander [he/him]]

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

I would like to get back to this bug again. How is it possible these days to set about:blank for a new tab that gets e.g manually opened via the tabs pane / view? The intent arguments -a android.intent.action.VIEW -d about:blank only seem to affect the initial start page but not any tab that gets opened later. Is that a GeckoView specific setting or related to the Android app embedding GeckoView?

That's correct: these intent arguments just impact the initial launch.

Note that we won't add support for that in Fennec anymore.

Nick maybe you can help or forward the question appropriately? Thanks.

My guess is that the App (Fenix, Reference Browser) controls what happens when a new tab is opened, and that we probably display some kind of "search/recall" UI like we do on iOS. So my guess is that you should start talking to the Fenix people more than the GV people. A quick skim of relevant GV sources strongly suggests that GV is doing the sensible thing and starting every tab with a navigation to about:blank. If you need to confirm, #geckoview on Matrix -- I expect many folks to know the details here, including :agi.

Good luck!

Comment 17

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

(In reply to Nick Alexander :nalexander [he/him] from comment #16)

My guess is that the App (Fenix, Reference Browser) controls what happens when a new tab is opened, and that we probably display some kind of "search/recall" UI like we do on iOS. So my guess is that you should start talking to the Fenix people more than the GV people. A quick skim of relevant GV sources strongly suggests that GV is doing the sensible thing and starting every tab with a navigation to about:blank. If you need to confirm, #geckoview on Matrix -- I expect many folks to know the details here, including :agi.

Thanks Nick. After a break of working on Android support for Marionette we would like to get back to this request.

Agi, could you maybe tell us which API we could use to create a new tab in a geckoview application which will then also present the about:blank page? A lot of tests meanwhile make use of this WebDriver API and it would be great to see it implemented for GeckoView. Thanks!

Comment 18

[[ex-Mozilla] Agi Sferro | :agi]

In general GeckoView does not deal with tabs, so we don't have an API for that.

What some places of GV do (e.g. tests) is pretending to be an extension and calling onNewTab, would that be a workable soulution for you?

I also wanna note that I don't think Fenix has a about:newtab equivalent, new tabs are always created to about:blank AFAIK.

Comment 19

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

Thanks Agi! I'll have a look in how we can get the tab opened. With bug 1723919 we will get better support for our tab handling wrappers and I might want to get this implemented via the MobileTabBrowser that you implemented a while ago. Also it's good to hear that about:newtab is not used.

Comment 20

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

Agi gave me some links on Friday. I'm going to dump these here for reference given that I won't be able to have a look in the next days.

https://searchfox.org/mozilla-central/rev/fc4d4a8d01b0e50d20c238acbb1739ccab317ebc/mobile/android/components/extensions/ext-android.js#620
https://searchfox.org/mozilla-central/rev/fc4d4a8d01b0e50d20c238acbb1739ccab317ebc/mobile/android/modules/test/AppUiTestDelegate.jsm#67
https://searchfox.org/mozilla-central/source/mobile/android/test_runner/src/main/assets/test-runner-support

Comment 21

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

We will enable the wdspec tests on Android first and then I hope to find the time to work on this issue hopefully soon.

Comment 22

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

Adding for triage so that we can discuss to get this included into our backlog and maybe get it even fixed in the M3 milestone. The bug should probably have at least 8 points.

Comment 23

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

I was trying to get opening a new tab implemented. As suggested by Agi I tried with a custom web extension by just copying what test-runner-support has. But I'm not able to get it installed most likely because I put it into the remote.jar file, whereby built-in webextensions might have to be located at resource://android/assets.

Agi, how could I package our extension so that it's getting correctly installed? Do I need some special packaging outside of Remote Agent to accomplish that? I'm somewhat blocked given that I cannot find any rules like in jar.mn files for Java packages. Thanks.

Comment 24

[[ex-Mozilla] Agi Sferro | :agi]

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

I was trying to get opening a new tab implemented. As suggested by Agi I tried with a custom web extension by just copying what test-runner-support has. But I'm not able to get it installed most likely because I put it into the remote.jar file, whereby built-in webextensions might have to be located at resource://android/assets.

Agi, how could I package our extension so that it's getting correctly installed? Do I need some special packaging outside of Remote Agent to accomplish that? I'm somewhat blocked given that I cannot find any rules like in jar.mn files for Java packages. Thanks.

I believe that as long as it's included in mobile/android/geckoview/src/main/assets it will work, no need to add jar.mn rules for that.

Comment 25

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

(In reply to Agi Sferro | :agi | [slow ni? rn sorry] | ⏰ PST | he/him from comment #24)

Agi, how could I package our extension so that it's getting correctly installed? Do I need some special packaging outside of Remote Agent to accomplish that? I'm somewhat blocked given that I cannot find any rules like in jar.mn files for Java packages. Thanks.

I believe that as long as it's included in mobile/android/geckoview/src/main/assets it will work, no need to add jar.mn rules for that.

Interesting. So there is no such assets folder for GeckoView in general, but as it looks like each application needs their own assets. That wouldn't be ideal for us given that we would like to ship it by default for any GeckoView application. Agi, do you think that having a mobile/android/geckoview/src/main/assets folder is fine? If yes, how / where it needs to be embedded into the build process? Maybe it's just /mobile/android/geckoview/build.gradle and everything in there will automatically be picked-up? Thanks a lot.

Comment 26

[Nick Alexander :nalexander [he/him]]

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

(In reply to Agi Sferro | :agi | [slow ni? rn sorry] | ⏰ PST | he/him from comment #24)

Agi, how could I package our extension so that it's getting correctly installed? Do I need some special packaging outside of Remote Agent to accomplish that? I'm somewhat blocked given that I cannot find any rules like in jar.mn files for Java packages. Thanks.

I believe that as long as it's included in mobile/android/geckoview/src/main/assets it will work, no need to add jar.mn rules for that.

Interesting. So there is no such assets folder for GeckoView in general, but as it looks like each application needs their own assets. That wouldn't be ideal for us given that we would like to ship it by default for any GeckoView application.

This is not quite correct: all libraries, and the consuming application, contribute their assets/ to the final application package. This accommodates library assets (such as this one) in a distributed manner, without consuming application authors having to stitch together their dependents' assets.

Agi, do you think that having a mobile/android/geckoview/src/main/assets folder is fine? If yes, how / where it needs to be embedded into the build process? Maybe it's just /mobile/android/geckoview/build.gradle and everything in there will automatically be picked-up? Thanks a lot.

Yes, this should be fine, and yes, it's all automatic. I.e., convention over configuration in this case.

Comment 27

[[ex-Mozilla] Agi Sferro | :agi]

I'm sorry I should've been more explicit, it's totally fine to create the assets folder and it will behave like Nick is describing.

Comment 28

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

Discussed with Agi on Element and using a ServiceWorkerDelegate instead might be more ergonomic given that it wouldn't require to have to install the WebExtension delegate in each and every application. I filed bug 1771012 for it.

Comment 29

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

Slightly updating the summary of the bug given that this affects both Marionette and WebDriver BiDi whereby the related code already lives in a shared module as used by both protocols.

Comment 30

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

Actually when opening tabs we have to also add support for closing tabs at the same time. Otherwise we would leave all the newly opened tabs open and won't be able to actually enable the wdspec tests.

Comment 31

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

Note that this bug is currently blocked on the GeckoView implementation (bug 1771012).

Comment 32

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

I started with enabling the WebDriver classic tests and most of them are passing which is great. But I can also see a couple of new failures coming up, or related to bugs that got closed incomplete a while ago.

Tomorrow I'm going to enable the feature as well for WebDriver BiDi + some refactoring so that Marionette will use the same TabManager.addTab() API.

The work is not that complex as initially thought but it's still quite a lot of meta data work necessary. As such I'm decreasing the points to 2.

Comment 33

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

Attachment: Bug 1506782 - [remote] Add support for opening and closing tabs on Android.

Depends on D156745

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/144d4be497c6
[remote] Add support for opening and closing tabs on Android. r=webdriver-reviewers,jdescottes

Comment 35

[Marian-Vasile Laza]

Backed out 2 changesets (Bug 1789357, Bug 1506782) for causing xpcshell failures on test_Capabilities.js.
Backout link
Push with failures <--> X1
Failure Log
Also Wd1 log

Comment 36

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

I missed to correctly rebase against the /webdriver/tests/bidi/browsing_context/load/load.py | test_new_context[window] test, which should also never run on Android.

Going to push again once it's fixed and verified locally.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc04dba3df30
[remote] Add support for opening and closing tabs on Android. r=webdriver-reviewers,jdescottes

Comment 38

[Sandor Molnar]

https://hg.mozilla.org/mozilla-central/rev/fc04dba3df30

Comment 39

[Julian Descottes [:jdescottes]]

Adding webdriver:relnote

Added support for opening and closing tabs on GeckoView applications (eg. Firefox for Android).