Commit 85d46806 authored by Kurt Horimoto's avatar Kurt Horimoto Committed by Commit Bot

[iOS] Move pdfs below headers when first loaded on iOS 11.

On iOS 12, resetting the WKScrollView.contentInset at the start of the
navigation pushes the content to {0, -contentInset.top}.  On iOS 11,
this does not occur.  This CL updates FullscreenWebStateObserver to
push the content below the toolbar the firs time a NavigationItem is
loaded with the shouldUseViewContentInset property set to YES.

This CL also updates MoveContentHelowHeader() to use the current top
toolbar inset rather than progress * extended toolbar height.  The
previous value was only working correctly because the function was
only called with a progress value of 1.0.

This CL re-enables FullscreenTestCase.testLongPDFInitialState, as
the behavior being tested is now fixed.

Bug: 904694, 903247
Change-Id: I134461e16054846a211a5476dd5a6694bd709002
Reviewed-on: https://chromium-review.googlesource.com/c/1371082
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615997}
parent c5b47189
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
void MoveContentBelowHeader(id<CRWWebViewProxy> proxy, FullscreenModel* model) { void MoveContentBelowHeader(id<CRWWebViewProxy> proxy, FullscreenModel* model) {
DCHECK(proxy); DCHECK(proxy);
DCHECK(model); DCHECK(model);
CGFloat topPadding = model->progress() * model->GetExpandedToolbarHeight(); CGFloat topPadding = model->current_toolbar_insets().top;
proxy.scrollViewProxy.contentOffset = CGPointMake(0, -topPadding); proxy.scrollViewProxy.contentOffset = CGPointMake(0, -topPadding);
if (!base::FeatureList::IsEnabled(web::features::kOutOfWebFullscreen)) { if (!base::FeatureList::IsEnabled(web::features::kOutOfWebFullscreen)) {
// With the fullscreen implementation living outside of web, this is no // With the fullscreen implementation living outside of web, this is no
......
...@@ -85,10 +85,6 @@ void AssertURLIs(const GURL& expectedURL) { ...@@ -85,10 +85,6 @@ void AssertURLIs(const GURL& expectedURL) {
// Verifies that the content offset of the web view is set up at the correct // Verifies that the content offset of the web view is set up at the correct
// initial value when initially displaying a PDF. // initial value when initially displaying a PDF.
- (void)testLongPDFInitialState { - (void)testLongPDFInitialState {
// TODO(crbug.com/904694): This test is failing on iOS11.
if (!base::ios::IsRunningOnIOS12OrLater())
EARL_GREY_TEST_DISABLED(@"Disabled on iOS 11.");
web::test::SetUpFileBasedHttpServer(); web::test::SetUpFileBasedHttpServer();
GURL URL = web::test::HttpServer::MakeUrl( GURL URL = web::test::HttpServer::MakeUrl(
"http://ios/testing/data/http_server_files/two_pages.pdf"); "http://ios/testing/data/http_server_files/two_pages.pdf");
...@@ -114,7 +110,12 @@ void AssertURLIs(const GURL& expectedURL) { ...@@ -114,7 +110,12 @@ void AssertURLIs(const GURL& expectedURL) {
} }
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
web::features::kBrowserContainerFullscreen) && web::features::kBrowserContainerFullscreen) &&
base::FeatureList::IsEnabled(web::features::kOutOfWebFullscreen)) { base::FeatureList::IsEnabled(web::features::kOutOfWebFullscreen) &&
base::ios::IsRunningOnIOS12OrLater()) {
// In the fullscreen browser implementation, the safe area is included in
// the top inset as well as the toolbar heights. Due to crbug.com/903635,
// however, this only occurs on iOS 12; pdf rendering does not correctly
// account for the safe area on iOS 11.
yOffset -= yOffset -=
chrome_test_util::GetCurrentWebState()->GetView().safeAreaInsets.top; chrome_test_util::GetCurrentWebState()->GetView().safeAreaInsets.top;
} }
......
...@@ -82,6 +82,11 @@ TEST_F(FullscreenWebStateListObserverTest, ObserveActiveWebState) { ...@@ -82,6 +82,11 @@ TEST_F(FullscreenWebStateListObserverTest, ObserveActiveWebState) {
std::unique_ptr<TestWebStateWithProxy> inserted_web_state = std::unique_ptr<TestWebStateWithProxy> inserted_web_state =
std::make_unique<TestWebStateWithProxy>(); std::make_unique<TestWebStateWithProxy>();
TestWebStateWithProxy* web_state = inserted_web_state.get(); TestWebStateWithProxy* web_state = inserted_web_state.get();
std::unique_ptr<web::TestNavigationManager> passed_navigation_manager =
std::make_unique<web::TestNavigationManager>();
web::TestNavigationManager* navigation_manager =
passed_navigation_manager.get();
web_state->SetNavigationManager(std::move(passed_navigation_manager));
web_state_list().InsertWebState(0, std::move(inserted_web_state), web_state_list().InsertWebState(0, std::move(inserted_web_state),
WebStateList::INSERT_ACTIVATE, WebStateList::INSERT_ACTIVATE,
WebStateOpener()); WebStateOpener());
...@@ -90,6 +95,9 @@ TEST_F(FullscreenWebStateListObserverTest, ObserveActiveWebState) { ...@@ -90,6 +95,9 @@ TEST_F(FullscreenWebStateListObserverTest, ObserveActiveWebState) {
SimulateFullscreenUserScrollForProgress(&model(), 0.5); SimulateFullscreenUserScrollForProgress(&model(), 0.5);
EXPECT_EQ(model().progress(), 0.5); EXPECT_EQ(model().progress(), 0.5);
// Simulate a navigation. The model should be reset by the observers. // Simulate a navigation. The model should be reset by the observers.
std::unique_ptr<web::NavigationItem> committed_item =
web::NavigationItem::Create();
navigation_manager->SetLastCommittedItem(committed_item.get());
web::FakeNavigationContext context; web::FakeNavigationContext context;
web_state->OnNavigationFinished(&context); web_state->OnNavigationFinished(&context);
EXPECT_FALSE(model().has_base_offset()); EXPECT_FALSE(model().has_base_offset());
......
...@@ -4,7 +4,9 @@ ...@@ -4,7 +4,9 @@
#import "ios/chrome/browser/ui/fullscreen/fullscreen_web_state_observer.h" #import "ios/chrome/browser/ui/fullscreen/fullscreen_web_state_observer.h"
#include "base/ios/ios_util.h"
#include "base/logging.h" #include "base/logging.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_content_adjustment_util.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_features.h" #import "ios/chrome/browser/ui/fullscreen/fullscreen_features.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_mediator.h" #import "ios/chrome/browser/ui/fullscreen/fullscreen_mediator.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_model.h" #import "ios/chrome/browser/ui/fullscreen/fullscreen_model.h"
...@@ -16,6 +18,7 @@ ...@@ -16,6 +18,7 @@
#include "ios/web/public/ssl_status.h" #include "ios/web/public/ssl_status.h"
#include "ios/web/public/url_util.h" #include "ios/web/public/url_util.h"
#import "ios/web/public/web_state/navigation_context.h" #import "ios/web/public/web_state/navigation_context.h"
#import "ios/web/public/web_state/page_display_state.h"
#import "ios/web/public/web_state/ui/crw_web_view_proxy.h" #import "ios/web/public/web_state/ui/crw_web_view_proxy.h"
#import "ios/web/public/web_state/web_state.h" #import "ios/web/public/web_state/web_state.h"
...@@ -91,6 +94,7 @@ void FullscreenWebStateObserver::DidFinishNavigation( ...@@ -91,6 +94,7 @@ void FullscreenWebStateObserver::DidFinishNavigation(
bool url_changed = web::GURLByRemovingRefFromGURL(navigation_url) != bool url_changed = web::GURLByRemovingRefFromGURL(navigation_url) !=
web::GURLByRemovingRefFromGURL(last_navigation_url_); web::GURLByRemovingRefFromGURL(last_navigation_url_);
last_navigation_url_ = navigation_url; last_navigation_url_ = navigation_url;
// Due to limitations in WKWebView's rendering, different MIME types must be // Due to limitations in WKWebView's rendering, different MIME types must be
// inset using different techniques: // inset using different techniques:
// - PDFs need to be inset using the scroll view's |contentInset| property or // - PDFs need to be inset using the scroll view's |contentInset| property or
...@@ -101,9 +105,23 @@ void FullscreenWebStateObserver::DidFinishNavigation( ...@@ -101,9 +105,23 @@ void FullscreenWebStateObserver::DidFinishNavigation(
bool force_content_inset = bool force_content_inset =
fullscreen::features::GetActiveViewportExperiment() == fullscreen::features::GetActiveViewportExperiment() ==
ViewportAdjustmentExperiment::CONTENT_INSET; ViewportAdjustmentExperiment::CONTENT_INSET;
web_state->GetWebViewProxy().shouldUseViewContentInset = bool is_pdf = web_state->GetContentsMimeType() == "application/pdf";
force_content_inset || bool use_content_inset = force_content_inset || is_pdf;
web_state->GetContentsMimeType() == "application/pdf"; id<CRWWebViewProxy> web_view_proxy = web_state->GetWebViewProxy();
web_view_proxy.shouldUseViewContentInset = use_content_inset;
// On iOS 12, resetting WKScrollView.contentInset at this point in the load
// will push the content down by the top inset. On iOS 11, however, this does
// not occur. Manually push the content below the toolbars the first time a
// page is loaded with the content inset setting enabled. The scroll offset
// of subsequent loads of this navigation will be set by the PageDisplayState.
web::NavigationItem* committed_item =
web_state->GetNavigationManager()->GetLastCommittedItem();
if (use_content_inset && !base::ios::IsRunningOnIOS12OrLater() &&
!committed_item->GetPageDisplayState().IsValid()) {
MoveContentBelowHeader(web_view_proxy, model_);
}
// Only reset the model for document-changing navigations or same-document // Only reset the model for document-changing navigations or same-document
// navigations that update the visible URL. // navigations that update the visible URL.
if (!navigation_context->IsSameDocument() || url_changed) if (!navigation_context->IsSameDocument() || url_changed)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment