Mixed content mode issue fix #8696
Conversation
|
By analyzing the blame information on this pull request, we identified @mkonicek and @nicklockwood to be potential reviewers. |
ghost
added
GH Review: review-needed
CLA Signed
|
I'm concerned about the security issue that pull request can do. The WebView will attempt to be compatible with the approach of a modern web browser with regard to mixed content. Some insecure content may be allowed to be loaded by a secure origin and other types of content will be blocked. The types of content are allowed or blocked may change release to release and are not explicitly defined. This mode is intended to be used by apps that are not in control of the content that they render but desire to operate in a reasonably secure environment. For highest security, apps are recommended to use IMO, this shouldn't be defined as |
|
Agree with the prop - let's have good defaults but also provide control. |
ghost
added
the
CLA Signed
| * start playing. The default value is `false`. | ||
| */ | ||
| mediaPlaybackRequiresUserAction: PropTypes.bool, | ||
|
|
There was a problem hiding this comment.
no-trailing-spaces: Trailing spaces not allowed.
| * Used on Android(5.0+), controls whether Mixed content mode is enabled or not | ||
| * @platform android | ||
| */ | ||
| enableMixedContentMode: PropTypes.bool, |
There was a problem hiding this comment.
can you rename this to mixedContentModeEnabled actually? other props in this component are named xxxEnabled
|
|
||
| @ReactProp(name = "enableMixedContentMode") | ||
| @ReactProp(name = "mixedContentModeEnabled") | ||
| public void setMixedContentMode(WebView view, boolean enabled) { |
There was a problem hiding this comment.
and name this setMixedContentModeEnabled
|
Looks good, thanks. Will merge after tests pass. |
ghost
added
the
CLA Signed
|
@ide Updated the pull request. |
ghost
added
the
CLA Signed
| @ReactProp(name = "mixedContentModeEnabled") | ||
| public void setMixedContentModeEnabled(WebView view, boolean enabled) { | ||
| if (enabled && Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { | ||
| view.getSettings().setMixedContentMode(WebSettings.MIXED_CONTENT_COMPATIBILITY_MODE); |
There was a problem hiding this comment.
Sorry looking at the Android API more closely, I think we want to expose the underlying API: always, never, compatibility as enums.
ghost
added
the
CLA Signed
|
@prabakarviji do you have any updates for this pull request? It's been a while since the last update so wanted to check in and see if you've looked at the requested changes. |
ghost
added
the
CLA Signed
|
@prabakarviji I'm going through all pull requests and noticed this one hasn't been updated in a while and the last comment requests changes. I'll close this pull request so it doesn't stay open indefinitely but please send a new one if you want to continue working on this. |
5 participants
PR for fix - Image not displaying due to mixed content in android(#8460)