Added fullscreen-video support #892

The-White-Fang · 2021-07-20T06:46:19Z

Platforms affected

Android

Motivation and Context

Adds full-screen video view support

Description

Updated InAppBrowser.java and InAppChromeClient.java
Added VideoEnabledWebChromeClient.java and VideoEnabledWebView.java
Code used to implement came from here which was originally derived from this.

Testing

Tested on Andoird 10 & Andoird 11

Checklist

I've run the tests to see all new and existing tests pass
I added automated test coverage as appropriate for this change
Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
I've updated the documentation if necessary

breautek

Hi there,

First I'd like to thank you for your time in preparing this PR. I was able to give this a glance over and there's a few main things that I want to point out.

There are new classes added that needs to be licensed under the Apache license for Apache to accept the code. If you have permission from the original author to re-license the code under the Apache License, then the Apache license headers needs to be added. Otherwise we cannot accept the Pr unfortunately.
This PR adds maintenance support for code going back as Android 3.0 (API 11). Cordova only supports API >= 22 and adding conditional logic to support anything less adds unnecessary complexity. I would like to see this simplified by removing any extra code required to support APIs < 22.

Kind regards,
Norman

breautek · 2021-07-20T14:23:31Z

src/android/InAppBrowser.java

         http://www.apache.org/licenses/LICENSE-2.0
-
       Unless required by applicable law or agreed to in writing,


The deletion of these lines will probably cause problems with our release process where it checks for proper license headers. Please revert these deletions

breautek · 2021-07-20T14:24:02Z

src/android/InAppBrowser.java

+                attrs.flags |= WindowManager.LayoutParams.FLAG_FULLSCREEN;
+                attrs.flags |= WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON;
+                window.setAttributes(attrs);
+                if (android.os.Build.VERSION.SDK_INT >= 14)


As of cordova-android@9, our supported min SDK is 22.

We don't need to have defensive checks here, and can assume that the SDK will be at least 22.

breautek · 2021-07-20T14:24:17Z

src/android/InAppBrowser.java

+                {
+                    //noinspection all
+                    int flags = View.SYSTEM_UI_FLAG_LOW_PROFILE;
+                    if (android.os.Build.VERSION.SDK_INT >= 16)


Same situation above.

breautek · 2021-07-20T14:24:47Z

src/android/InAppBrowser.java

+                attrs.flags &= ~WindowManager.LayoutParams.FLAG_FULLSCREEN;
+                attrs.flags &= ~WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON;
+                window.setAttributes(attrs);
+                if (android.os.Build.VERSION.SDK_INT >= 14)


See comment above

breautek · 2021-07-20T14:25:42Z

src/android/InAppBrowser.java

+                    }
+
+                    // For Android 3.0+
+                    public void openFileChooser(ValueCallback<Uri> uploadMsg, String acceptType)


We don't need to support android 3.x

breautek · 2021-07-20T14:25:57Z

src/android/InAppBrowser.java

                        return true;
                    }
+
+                    // For Android 4.1+
+                    public void openFileChooser(ValueCallback<Uri> uploadMsg, String acceptType, String capture)


We don't need to support android 4.x

breautek · 2021-07-20T14:31:49Z

src/android/VideoEnabledWebChromeClient.java

+ * - The invoking activity must call VideoEnabledWebChromeClient's onBackPressed() inside of its own onBackPressed().
+ * - Tested in Android API levels 8-19. Only tested on http://m.youtube.com.
+ *
+ * @author Cristian Perez (http://cpr.name)


Are you Christian Perez? If not, do you have permission to submit this code under the Apache license?

If yes, then the Apache license needs to be added to this file.

This seems copy/pasted from https://github.com/cprcrack/VideoEnabledWebView

This seems copy/pasted from https://github.com/cprcrack/VideoEnabledWebView

It actually is. I needed those changes in my current project so I forked the latest repo and modified it so, if anyone else wants to use it. They can.

breautek · 2021-07-20T14:32:08Z

src/android/VideoEnabledWebView.java

+ * - Javascript is enabled by default and must not be disabled with getSettings().setJavaScriptEnabled(false).
+ * - setWebChromeClient() must be called before any loadData(), loadDataWithBaseURL() or loadUrl() method.
+ *
+ * @author Cristian Perez (http://cpr.name)


Same concern above regarding licensing.

breautek · 2021-07-20T14:37:33Z

src/android/InAppBrowser.java

@@ -132,8 +133,10 @@ Licensed to the Apache Software Foundation (ASF) under one
    private boolean mediaPlaybackRequiresUserGesture = false;
    private boolean shouldPauseInAppBrowser = false;
    private boolean useWideViewPort = true;
-    private ValueCallback<Uri[]> mUploadCallback;
+    private ValueCallback<Uri> mUploadCallback;
+    private ValueCallback<Uri[]> mUploadCallbackLollipop;


Removing code that handles API < 21 should also eliminate the need to have two different class members to handle different API levels.

breautek · 2021-07-20T14:37:49Z

src/android/InAppBrowser.java

    private final static int FILECHOOSER_REQUESTCODE = 1;
+    private final static int FILECHOOSER_REQUESTCODE_LOLLIPOP = 2;


Same thing here; see comment above.

The-White-Fang · 2021-07-20T15:48:45Z

Hi there,

First I'd like to thank you for your time in preparing this PR. I was able to give this a glance over and there's a few main things that I want to point out.

There are new classes added that needs to be licensed under the Apache license for Apache to accept the code. If you have permission from the original author to re-license the code under the Apache License, then the Apache license headers needs to be added. Otherwise we cannot accept the Pr unfortunately.

This PR adds maintenance support for code going back as Android 3.0 (API 11). Cordova only supports API >= 22 and adding conditional logic to support anything less adds unnecessary complexity. I would like to see this simplified by removing any extra code required to support APIs < 22.

Kind regards,
Norman

Hey Norman,
I would gladly update the code as you have requested. But it might take me some time. Hope you understand.

breautek · 2021-07-20T16:05:15Z

Hi there,
First I'd like to thank you for your time in preparing this PR. I was able to give this a glance over and there's a few main things that I want to point out.

There are new classes added that needs to be licensed under the Apache license for Apache to accept the code. If you have permission from the original author to re-license the code under the Apache License, then the Apache license headers needs to be added. Otherwise we cannot accept the Pr unfortunately.

This PR adds maintenance support for code going back as Android 3.0 (API 11). Cordova only supports API >= 22 and adding conditional logic to support anything less adds unnecessary complexity. I would like to see this simplified by removing any extra code required to support APIs < 22.

Kind regards,
Norman

Hey Norman,
I would gladly update the code as you have requested. But it might take me some time. Hope you understand.

To hopefully save you some time, I think it would be best to analyse and see if you can fulfil the original requirements to solve #320 without using the code by Cristian Perez. The original code appears to be licensed under MIT, and while that may allow you to copy the code for your own use, all code submitted to Apache needs to be licensed under the Apache license. So either the code By Christian needs to be removed, or we need obtain written permission to be able to license that code under Apache. If this cannot be done, then we should close this PR.

Added fullscreen-video support

74e32d2

breautek requested changes Jul 20, 2021

View reviewed changes

= added 2 commits October 5, 2021 15:21

updated plugin.xml

7a32aea

beforeload firing on alternate requests fixed #bug686

6652636

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added fullscreen-video support #892

Added fullscreen-video support #892

The-White-Fang commented Jul 20, 2021

breautek left a comment

breautek Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

timbru31 Jul 20, 2021

The-White-Fang Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

breautek Jul 20, 2021

The-White-Fang commented Jul 20, 2021

breautek commented Jul 20, 2021

		http://www.apache.org/licenses/LICENSE-2.0

		Unless required by applicable law or agreed to in writing,

		private final static int FILECHOOSER_REQUESTCODE = 1;
		private final static int FILECHOOSER_REQUESTCODE_LOLLIPOP = 2;

Added fullscreen-video support #892

Are you sure you want to change the base?

Added fullscreen-video support #892

Conversation

The-White-Fang commented Jul 20, 2021

Platforms affected

Motivation and Context

Description

Testing

Checklist

breautek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The-White-Fang commented Jul 20, 2021

breautek commented Jul 20, 2021