What can Firefox provide with PIP mode supported from Android framework O? * Video playback in PIP mode? * Float browsing in PIP mode on tablet?

There is also an ongoing Web API standardization effort [1] which IMHO should be taken into consideration too. BTW, looks like Chrome already has it for Android O [2]. [1] https://github.com/WICG/picture-in-picture [2] https://developers.google.com/web/updates/2017/09/picture-in-picture

In regards to Max's comment, I think we can do PIP for videos, I started to work on this. The new Picture-In-Picture mode is mainly intended for playing video on top of other apps and so this functionality is not really suitable for browsing in PIP mode. More so because although we can minimize the entire activity we have no control over the dimensions of the window, which will be quite small and also because there is limited interaction possible through that small window.

Comment on attachment 8985949 [details] Bug 1349523 - Add support for playing videos in Picture-in-picture mode; https://reviewboard.mozilla.org/r/251430/#review258490 ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:255 (Diff revision 1) > > public ViewGroup mBrowserChrome; > public ViewFlipper mActionBarFlipper; > public ActionModeCompatView mActionBar; > private VideoPlayer mVideoPlayer; > + private PictureInPictureController pipController; `mPipController` ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1360 (Diff revision 1) > + if (startingIntentAfterPip != null) { > + getApplication().startActivity(startingIntentAfterPip); > + startingIntentAfterPip = null; > + } else { > + // Android's status bar would be shown otherwise > + ActivityUtils.setFullScreen(this, true); `setFullScreen(this, true)` shows the status bar? ::: mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java:75 (Diff revision 1) > private int coverSize; > > /** > * Internal state of MediaControlService, to indicate it is playing media, or paused...etc. > */ > - private State mMediaState = State.STOPPED; > + private static State mMediaState = State.STOPPED; Rename to `sMediaState` ::: mobile/android/base/java/org/mozilla/gecko/media/PictureInPictureController.java:32 (Diff revision 1) > + private static final String PAUSE_ACTION_TITLE = "Pause"; > + private static final String PAUSE_ACTION_DESCRIPTION = "Pause playing"; > + private static final String PLAY_ACTION_TITLE = "Play"; > + private static final String PLAY_ACTION_DESCRIPTION = "Resume playing"; I think these should be localized ::: mobile/android/base/java/org/mozilla/gecko/media/PictureInPictureController.java:65 (Diff revision 1) > + return isInPipMode; > + } > + > + private boolean shouldTryPipMode() { > + if (!AppConstants.Versions.feature26Plus) { > + Log.d(LOGTAG, "Picture In Picture is only available on Oreo+"); Put logging inside `if (DEBUG) { ... }`

Comment on attachment 8985949 [details] Bug 1349523 - Add support for playing videos in Picture-in-picture mode; https://reviewboard.mozilla.org/r/251430/#review258490 > `setFullScreen(this, true)` shows the status bar? The status bar is shown after returning from the Picture-in-picture mode to the fullscreen playing video. I call setFullscreen(..) to hide the status bar and so to offer the same fullscreen video experience that the user had before entering in Picture-in-picture mode. > I think these should be localized Thanks! At first I wasn't sure exactly where would those be used by after more testing I see they are needed for when having an accesibility service that provides spoken feedback started.

Thanks Jim for the review! Although initially I wasn't sure how those String from `PictureInPictureController` would be exposed to the user I tested with an accessibility service on and found that indeed they are used when providing spoken feedback about the play/pause buttons of the Picture-in-picture window. Another interesting thing that I had found is that having such an accessibility service enabled on a Samsung phone and trying to enter Picture-inPicture mode would crash the app with: > java.lang.IllegalStateException: enterPictureInPictureMode: Device doesn't support picture-in-picture mode. This is happening on a Samsung S8 and on a Samsung S7 with stock Android 8.0. Tried to reproduce with other apps and found that: - VLC is crashing - Chrome and Maps are swallowing the error, they show it in logs but prevent crashing. Tried to reproduce on a Huawei Nexus 6P with Android 8.1 and on a Nokia 7 with Android P beta 2 and there would no such issues, the apps would enter Picture-in-picture mode as expected. Because of this, before entering Picture-in-picture mode I have added a new check to see if currently there is an accessibility service running on a Samsung device, case in which we would not enter Picture-in-picture mode. Also, in the case that there are some other similar issues with other devices, the app would catch and log the exception but not crash. With this changes in mind I would appreciate if you could take a second look at the patch.

Comment on attachment 8985949 [details] Bug 1349523 - Add support for playing videos in Picture-in-picture mode; https://reviewboard.mozilla.org/r/251430/#review258746 Looks good! Thanks!

Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/add212ee1118 Add support for playing videos in Picture-in-picture mode; r=jchen

Verified that videos work in PIP mode on Nightly 63. Devices: Google Pixel (Android P Beta) Samsung Galaxy S8 (Android 8.0) Samsung S7 Edge (Android 8.0) OnePlus 5T (Android 8.0)