Add support for opening and closing of tabs for GeckoView applications (Marionette and WebDriver BiDi)
Categories
(Remote Protocol :: Agent, task, P1)
Tracking
(firefox105 wontfix, firefox106 fixed)
People
(Reporter: whimboo, Assigned: whimboo)
References
(Blocks 4 open bugs)
Details
(Whiteboard: [webdriver:m4][webdriver:relnote])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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.
I don't really have knowledge of how this works in the Fennec UI. Someone on Susheel's team should be able to help.
As a P3 this will show up in triage at some point.
Assignee | ||
Comment 3•6 years ago
|
||
(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.
Sorry - can you help me with priority here? What does this work block, and how important is it?
Assignee | ||
Comment 5•6 years ago
|
||
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.
Liz how can we prioritize this against our other Fennec needs?
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?
Assignee | ||
Comment 8•6 years ago
|
||
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.
Assignee | ||
Comment 9•5 years ago
|
||
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:
Where are those preferences set? James, could you please help? Thanks
Assignee | ||
Comment 10•5 years ago
|
||
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.
Assignee | ||
Updated•5 years ago
|
Comment 11•5 years ago
|
||
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.
Assignee | ||
Updated•5 years ago
|
Yeah, comment #11 captures the problem -- an intent argument is a good way to add this kind of thing to Fennec.
Assignee | ||
Comment 13•5 years ago
|
||
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?
(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.
Assignee | ||
Comment 15•4 years ago
|
||
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•4 years ago
|
||
(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!
Assignee | ||
Comment 17•3 years ago
|
||
(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•3 years ago
|
||
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.
Assignee | ||
Comment 19•3 years ago
|
||
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.
Assignee | ||
Comment 20•3 years ago
|
||
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
Assignee | ||
Comment 21•3 years ago
|
||
We will enable the wdspec tests on Android first and then I hope to find the time to work on this issue hopefully soon.
Assignee | ||
Comment 22•2 years ago
|
||
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.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 23•2 years ago
|
||
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•2 years ago
|
||
(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 atresource://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.
Assignee | ||
Comment 25•2 years ago
|
||
(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 addjar.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•2 years ago
|
||
(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 addjar.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•2 years ago
|
||
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.
Assignee | ||
Comment 28•2 years ago
|
||
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.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 29•2 years ago
|
||
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.
Assignee | ||
Comment 30•2 years ago
|
||
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.
Assignee | ||
Comment 31•2 years ago
|
||
Note that this bug is currently blocked on the GeckoView implementation (bug 1771012).
Updated•2 years ago
|
Assignee | ||
Comment 32•2 years ago
|
||
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.
Updated•2 years ago
|
Comment 34•2 years ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/144d4be497c6 [remote] Add support for opening and closing tabs on Android. r=webdriver-reviewers,jdescottes
Comment 35•2 years ago
|
||
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
Assignee | ||
Comment 36•2 years ago
|
||
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•2 years ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fc04dba3df30 [remote] Add support for opening and closing tabs on Android. r=webdriver-reviewers,jdescottes
Comment 38•2 years ago
|
||
bugherder |
Updated•2 years ago
|
Comment 39•2 years ago
|
||
Adding webdriver:relnote
Added support for opening and closing tabs on GeckoView applications (eg. Firefox for Android).
Description
•