Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Android 4.1-4.3 WebView source baseUrl bug #13789

Closed
wants to merge 2 commits into from

Conversation

@Etheryte
Copy link
Contributor

@Etheryte Etheryte commented May 5, 2017

Motivation

Resolves #11753 and #9835.

Description

Android versions 4.1-4.3 don't understand the MIME type text/html; charset=utf-8 and default to text/plain instead, rendering the content as an unparsed HTML string. Since the encoding is already set and passed separately, removing it from the MIME type has no negative effects.

The same fix has already been discussed, successfully tested and incorporated in NativeScript/NativeScript#1038.

Resolves facebook#11753. Android versions 4.1-4.3 don't understand the MIME type 'text/html; charset=utf-8' and default to 'text/plain' instead, rendering the content as an unparsed HTML string. Since the encoding is already set and passed separately, removing it from the MIME type has no negative effects.
Etheryte pushed a commit to Etheryte/react-native that referenced this issue May 14, 2017
@shergin shergin added the Android label Jun 2, 2017
Etheryte pushed a commit to Etheryte/react-native that referenced this issue Jun 7, 2017
@Exilz
Copy link

@Exilz Exilz commented Jul 3, 2017

Can someone review this PR so we can see it merged in the near future ? We had to make the exact same change on react-native, this is a quite critical bug for us.

Loading

@stereodenis
Copy link
Contributor

@stereodenis stereodenis commented Jul 4, 2017

@mkonicek how to help you get it merged?

Loading

@chung2014
Copy link

@chung2014 chung2014 commented Jul 6, 2017

will this fix merge to master?

Loading

@vikkalen
Copy link

@vikkalen vikkalen commented Jul 27, 2017

the scope of the property HTML_MIME_TYPE has recently changed from "private" to "protected". See commit e2c87b5. You should probably fix your pull request.

Loading

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Aug 27, 2017

@Etheryte I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

Loading

@Etheryte
Copy link
Contributor Author

@Etheryte Etheryte commented Aug 28, 2017

@facebook-github-bot This pull request is still relevant, I've merged master into the branch. I don't know who to highlight with this request.

Loading

@pull-bot
Copy link

@pull-bot pull-bot commented Aug 28, 2017

Warnings
⚠️

📋 Test Plan - This PR appears to be missing a Test Plan.

@facebook-github-bot label Needs more information

Generated by 🚫 dangerJS

Loading

Etheryte pushed a commit to Etheryte/react-native that referenced this issue Sep 25, 2017
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Sep 27, 2017

@Etheryte I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

Loading

@chirag04
Copy link
Contributor

@chirag04 chirag04 commented Sep 27, 2017

Loading

@shergin
Copy link
Contributor

@shergin shergin commented Sep 29, 2017

I can merge this on Monday.

Loading

@Crash--
Copy link
Contributor

@Crash-- Crash-- commented Oct 3, 2017

+1 to merge this PR. ATM, we've to override the setSource() methods in order to be able to display HTML on Android 4.2.2 :(

Loading

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

10 participants