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.

Sign up for GitHub

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

Jump to bottom

[Android] Webview: Fix broken fullscreen modals due to zero body height. #8830

Closed
wants to merge 4 commits into from
Closed

[Android] Webview: Fix broken fullscreen modals due to zero body height. #8830

wants to merge 4 commits into from

Conversation

danielbraun
Copy link
Contributor

Supersedes PR #8536
Fixes #5211

JavaScript plugins such as Fotorama are broken when attempting use its fullscreen feature.

If there's an absolute HTML element with 100% height under , its height is 0 when rendered in the Android WebView.

This commit fixes it.

@danielbraun danielbraun mentioned this pull request Jul 16, 2016
Closed
@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @mkonicek and @sathyapriya-31 to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 16, 2016
@satya164 satya164 changed the title Webview: Fixed broken fullscreen modals due to zero body height. [Android] Webview: Fix broken fullscreen modals due to zero body height. Jul 16, 2016
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 16, 2016
@satya164
Copy link
Contributor

Seems Android tests are failing due to some error with NativeRunnable.

cc @bestander

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 16, 2016
@bestander
Copy link
Contributor

Yeah, it is based on a broken trunk.
@danielbraun, could you please rebase on master?

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 18, 2016
@danielbraun
Copy link
Contributor Author

There's an exception in the test runner, unrelated to the PR. I've seen the same error exception occur in other PR's as well.

@bestander
Copy link
Contributor

Can you rebase then?
It should be fine now

On Monday, 18 July 2016, Daniel Braun notifications@github.com wrote:

There's an exception in the test runner, unrelated to the PR. I've seen
the same error exception occur in other PR's as well.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8830 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACBdWJS74OIN4H-q2wwtZNh73g-L3JDnks5qW_6hgaJpZM4JOCq_
.

@danielbraun
Copy link
Contributor Author

Seems the tests passed.

@bestander
Copy link
Contributor

@facebook-github-bot shipit

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 22, 2016
@ghost
Copy link

ghost commented Jul 22, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2016
@bestander
Copy link
Contributor

@danielbraun could you rebase please?
The patch does not apply

@ghost ghost added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jul 22, 2016
@ghost
Copy link

ghost commented Jul 23, 2016

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 23, 2016
@danielbraun
Copy link
Contributor Author

try again? I solved the conflicts

@bestander
Copy link
Contributor

@facebook-github-bot shipit

@ghost ghost added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jul 26, 2016
@ghost
Copy link

ghost commented Jul 26, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 26, 2016
@ghost
Copy link

ghost commented Jul 27, 2016

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@ghost ghost added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jul 27, 2016
@bestander
Copy link
Contributor

@facebook-github-bot import

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 27, 2016
@bestander
Copy link
Contributor

We have some infrastructure errors, I'll deal with that

@bestander
Copy link
Contributor

@facebook-github-bot shipit

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 27, 2016
@bestander
Copy link
Contributor

@facebook-github-bot shipit

@ghost ghost added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jul 27, 2016
@ghost
Copy link

ghost commented Jul 27, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 27, 2016
@ghost ghost closed this in 1bb1385 Jul 27, 2016
bartolkaruza pushed a commit to immidi/react-native that referenced this pull request Aug 3, 2016
@danielbraun
Webview: Fix broken fullscreen modals due to zero body height.
af332fc 
Summary:
Supersedes PR facebook#8536
Fixes facebook#5211

JavaScript plugins such as Fotorama are broken when attempting use its fullscreen feature.

If there's an absolute HTML element with 100% height under <body>, its height is 0 when rendered in the Android WebView.

This commit fixes it.
Closes facebook#8830

Reviewed By: bestander

Differential Revision: D3632821

Pulled By: jamesgpearce

fbshipit-source-id: c185bcd30d1d214a357d0d8552d61d0ddfa5e6c6
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
@danielbraun
Webview: Fix broken fullscreen modals due to zero body height.
7ad898c 
Summary:
Supersedes PR facebook#8536
Fixes facebook#5211

JavaScript plugins such as Fotorama are broken when attempting use its fullscreen feature.

If there's an absolute HTML element with 100% height under <body>, its height is 0 when rendered in the Android WebView.

This commit fixes it.
Closes facebook#8830

Reviewed By: bestander

Differential Revision: D3632821

Pulled By: jamesgpearce

fbshipit-source-id: c185bcd30d1d214a357d0d8552d61d0ddfa5e6c6
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Android WebView] can't load url
4 participants
@danielbraun @facebook-github-bot @satya164 @bestander