1815918 - (custom-tabs-refresh) [meta] Custom Tabs refresh

Status: NEW

(Firefox for Android :: Tabs, task, P3)

Description

[Jonathan Almeida [:jonalmeida]]

This meta is to track a range of issues and feature requests within Custom Tabs that users have experienced or new features that Custom Tab providers should be supporting.

Comment 1

[Jonathan Almeida [:jonalmeida]]

Meta bugs are not enhancements because they are meant to hold other defect, features, and tasks.

Comment 2

[killercontact1.7.4.0]

Please have a look at https://bugzilla.mozilla.org/show_bug.cgi?id=1861616 as it is indeed a quality of life issue

Comment 3

[[:owlish] 🦉 PST]

Nika discovered an issue with our Custom Tabs in general, and the handling of Web Extension APIs in particular.

When we handle browserAction, pageAction etc Web Extension API in AC and Fenix, it looks like we are following the Custom Tab flow by initializing a GeckoSession when registering a delegate for a web extension. Then we use that Gecko Session to load URLs in a popup[1][2]. However, it looks like if there is some sort of navigation in those popups, we might be initializing a new GeckoSession and open it[3], and then drop it, because it is not used anywhere anymore.

[1] https://github.com/mozilla-mobile/firefox-android/blob/2fb5842a7ed9b8dcd4b494c23eb5abdb3a81282f/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt#L186
[2] https://github.com/mozilla-mobile/firefox-android/blob/2fb5842a7ed9b8dcd4b494c23eb5abdb3a81282f/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/addons/WebExtensionActionPopupActivity.kt#L90
[3] https://github.com/mozilla-mobile/firefox-android/blob/2fb5842a7ed9b8dcd4b494c23eb5abdb3a81282f/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt#L1206-L1216

This looks like a bug. The case would seem to be a navigation to a link with target="_blank" from a popup. Arturo, can you please confirm - would we really end up with two GeckoSessions in that case? (one from the web extension fragment's state, and another one initialized in the onNewSession override)

Comment 4

[Arturo Mejia [:amejia]]

(In reply to [:owlish] 🦉 PST from comment #3)

Nika discovered an issue with our Custom Tabs in general, and the handling of Web Extension APIs in particular.

When we handle browserAction, pageAction etc Web Extension API in AC and Fenix, it looks like we are following the Custom Tab flow by initializing a GeckoSession when registering a delegate for a web extension. Then we use that Gecko Session to load URLs in a popup[1][2]. However, it looks like if there is some sort of navigation in those popups, we might be initializing a new GeckoSession and open it[3], and then drop it, because it is not used anywhere anymore.

[1] https://github.com/mozilla-mobile/firefox-android/blob/2fb5842a7ed9b8dcd4b494c23eb5abdb3a81282f/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt#L186
[2] https://github.com/mozilla-mobile/firefox-android/blob/2fb5842a7ed9b8dcd4b494c23eb5abdb3a81282f/android-components/samples/browser/src/main/java/org/mozilla/samples/browser/addons/WebExtensionActionPopupActivity.kt#L90
[3] https://github.com/mozilla-mobile/firefox-android/blob/2fb5842a7ed9b8dcd4b494c23eb5abdb3a81282f/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt#L1206-L1216

This looks like a bug. The case would seem to be a navigation to a link with target="_blank" from a popup. Arturo, can you please confirm - would we really end up with two GeckoSessions in that case? (one from the web extension fragment's state, and another one initialized in the onNewSession override)

Thanks bringing this up!

I'm not that familiar with the initialization of the session in the AddonPopupBaseFragment, but agree it looks really odd that we are following the Custom Tab flow to initialize the session. Would you mind filing a bug for us to investigate? Please lets include some STR with a test small sample case, where it has the expected behaviour and the actual wrong behaviour, so we can put a fix if it's needed. Thank you again!