Commit 8ecc5619 authored by Robbie Gibson's avatar Robbie Gibson Committed by Commit Bot

[iOS] Persist Google AMP pages text zoom settings separately

As Text Zoom uses the URL host as its persistence key, Google AMP urls
are all persisted under "www.google.com," which is not ideal. To fix
this, if the url is an AMP url, identified using the heuristic that all
AMP urls are www.<google domain>.com/amp/, the persistence key is
www.google.com/amp.

Also, when navigating to an AMP page from Google Search/News, no actual
navigation takes place. Instead, the page is presented using Javascript.
To make sure that the newly "loaded" page has the correct zoom level,
the zoom level is set in DidFinishNavigation if the page navigated to
is a Google AMP page.

Bug: 1063836
Change-Id: I2282cbacf938eaaf0f7d03925dbe9e60b8fee213
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2119541Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Cr-Commit-Position: refs/heads/master@{#753625}
parent c116daa1
...@@ -76,15 +76,21 @@ class FontSizeTabHelper : public web::WebStateObserver, ...@@ -76,15 +76,21 @@ class FontSizeTabHelper : public web::WebStateObserver,
// 150%) taking all sources into account (system level and user zoom). // 150%) taking all sources into account (system level and user zoom).
int GetFontSize() const; int GetFontSize() const;
// Set the zoom level correctly for a new navigation.
void NewPageZoom();
PrefService* GetPrefService() const; PrefService* GetPrefService() const;
// Returns the new multiplier after zooming in the given direction. Returns
// nullopt if it is impossible to zoom in the given direction;
base::Optional<double> NewMultiplierAfterZoom(Zoom zoom) const; base::Optional<double> NewMultiplierAfterZoom(Zoom zoom) const;
// Returns the current user zoom multiplier (i.e. not counting any additional // Returns the current user zoom multiplier (i.e. not counting any additional
// zoom due to the system accessibility settings). // zoom due to the system accessibility settings).
double GetCurrentUserZoomMultiplier() const; double GetCurrentUserZoomMultiplier() const;
void StoreCurrentUserZoomMultiplier(double multiplier); void StoreCurrentUserZoomMultiplier(double multiplier);
std::string GetCurrentUserZoomMultiplierKey() const; std::string GetCurrentUserZoomMultiplierKey() const;
std::string GetUserZoomMultiplierKeyUrlPart() const;
bool IsGoogleCachedAMPPage() const;
bool tab_helper_has_zoomed_ = false; bool tab_helper_has_zoomed_ = false;
// web::WebStateObserver overrides: // web::WebStateObserver overrides:
...@@ -92,6 +98,10 @@ class FontSizeTabHelper : public web::WebStateObserver, ...@@ -92,6 +98,10 @@ class FontSizeTabHelper : public web::WebStateObserver,
web::WebState* web_state, web::WebState* web_state,
web::PageLoadCompletionStatus load_completion_status) override; web::PageLoadCompletionStatus load_completion_status) override;
void WebStateDestroyed(web::WebState* web_state) override; void WebStateDestroyed(web::WebState* web_state) override;
void DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* context) override;
void WebFrameDidBecomeAvailable(web::WebState* web_state,
web::WebFrame* web_frame) override;
// Observer id returned by registering at NSNotificationCenter. // Observer id returned by registering at NSNotificationCenter.
id content_size_did_change_observer_ = nil; id content_size_did_change_observer_ = nil;
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#import "base/strings/sys_string_conversions.h" #import "base/strings/sys_string_conversions.h"
#import "base/values.h" #import "base/values.h"
#include "components/google/core/common/google_util.h"
#include "components/pref_registry/pref_registry_syncable.h" #include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h" #include "components/prefs/scoped_user_pref_update.h"
...@@ -233,6 +234,35 @@ void FontSizeTabHelper::PageLoaded( ...@@ -233,6 +234,35 @@ void FontSizeTabHelper::PageLoaded(
web::WebState* web_state, web::WebState* web_state,
web::PageLoadCompletionStatus load_completion_status) { web::PageLoadCompletionStatus load_completion_status) {
DCHECK_EQ(web_state, web_state_); DCHECK_EQ(web_state, web_state_);
NewPageZoom();
}
void FontSizeTabHelper::DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* context) {
// When navigating to a Google AMP Cached page, the navigation occurs via
// Javascript, so handle that separately.
if (IsGoogleCachedAMPPage()) {
NewPageZoom();
}
}
void FontSizeTabHelper::WebFrameDidBecomeAvailable(web::WebState* web_state,
web::WebFrame* web_frame) {
// Make sure that any new web frame starts with the correct zoom level.
DCHECK_EQ(web_state, web_state_);
int size = GetFontSize();
// Prevent any zooming errors by only zooming when necessary. This is mostly
// when size != 100, but if zooming has happened before, then zooming to 100
// may be necessary to reset a previous page to the correct zoom level.
if (tab_helper_has_zoomed_ || size != 100) {
std::vector<base::Value> parameters;
parameters.push_back(base::Value(size));
web_frame->CallJavaScriptFunction("accessibility.adjustFontSize",
parameters);
}
}
void FontSizeTabHelper::NewPageZoom() {
int size = GetFontSize(); int size = GetFontSize();
// Prevent any zooming errors by only zooming when necessary. This is mostly // Prevent any zooming errors by only zooming when necessary. This is mostly
// when size != 100, but if zooming has happened before, then zooming to 100 // when size != 100, but if zooming has happened before, then zooming to 100
...@@ -252,7 +282,15 @@ std::string FontSizeTabHelper::GetCurrentUserZoomMultiplierKey() const { ...@@ -252,7 +282,15 @@ std::string FontSizeTabHelper::GetCurrentUserZoomMultiplierKey() const {
std::string content_size_category = base::SysNSStringToUTF8( std::string content_size_category = base::SysNSStringToUTF8(
UIApplication.sharedApplication.preferredContentSizeCategory); UIApplication.sharedApplication.preferredContentSizeCategory);
return base::StringPrintf("%s.%s", content_size_category.c_str(), return base::StringPrintf("%s.%s", content_size_category.c_str(),
web_state_->GetLastCommittedURL().host().c_str()); GetUserZoomMultiplierKeyUrlPart().c_str());
}
std::string FontSizeTabHelper::GetUserZoomMultiplierKeyUrlPart() const {
if (IsGoogleCachedAMPPage()) {
return web_state_->GetLastCommittedURL().host().append("/amp");
}
return web_state_->GetLastCommittedURL().host();
} }
double FontSizeTabHelper::GetCurrentUserZoomMultiplier() const { double FontSizeTabHelper::GetCurrentUserZoomMultiplier() const {
...@@ -273,4 +311,21 @@ void FontSizeTabHelper::StoreCurrentUserZoomMultiplier(double multiplier) { ...@@ -273,4 +311,21 @@ void FontSizeTabHelper::StoreCurrentUserZoomMultiplier(double multiplier) {
} }
} }
bool FontSizeTabHelper::IsGoogleCachedAMPPage() const {
// All google AMP pages have URL in the form "https://google_domain/amp/..."
// This method checks that this is strictly the case.
const GURL& url = web_state_->GetLastCommittedURL();
if (!url.is_valid() || !url.SchemeIs(url::kHttpsScheme)) {
return false;
}
if (!google_util::IsGoogleDomainUrl(
url, google_util::DISALLOW_SUBDOMAIN,
google_util::DISALLOW_NON_STANDARD_PORTS) ||
url.path().compare(0, 5, "/amp/") != 0) {
return false;
}
return true;
}
WEB_STATE_USER_DATA_KEY_IMPL(FontSizeTabHelper) WEB_STATE_USER_DATA_KEY_IMPL(FontSizeTabHelper)
...@@ -333,3 +333,32 @@ TEST_F(FontSizeTabHelperTest, CanZoomContent) { ...@@ -333,3 +333,32 @@ TEST_F(FontSizeTabHelperTest, CanZoomContent) {
web_state_.SetContentIsHTML(true); web_state_.SetContentIsHTML(true);
EXPECT_TRUE(font_size_tab_helper->CurrentPageSupportsTextZoom()); EXPECT_TRUE(font_size_tab_helper->CurrentPageSupportsTextZoom());
} }
TEST_F(FontSizeTabHelperTest, GoogleCachedAMPPageHasSeparateKey) {
// First, zoom in on a regular Google url.
GURL google_url("https://www.google.com/");
web_state_.SetCurrentURL(google_url);
FontSizeTabHelper* font_size_tab_helper =
FontSizeTabHelper::FromWebState(&web_state_);
font_size_tab_helper->UserZoom(ZOOM_IN);
std::string google_pref_key =
ZoomMultiplierPrefKey(preferred_content_size_category_, google_url);
// Next, zoom out on a Google AMP url and make sure both states are saved
// separately.
GURL google_amp_url("https://www.google.com/amp/s/www.france24.com");
web_state_.SetCurrentURL(google_amp_url);
font_size_tab_helper->UserZoom(ZOOM_OUT);
// Google AMP pages use a different key for the URL part.
std::string google_amp_pref_key = base::StringPrintf(
"%s.%s",
base::SysNSStringToUTF8(preferred_content_size_category_).c_str(),
"www.google.com/amp");
EXPECT_NE(google_pref_key, google_amp_pref_key);
const base::Value* pref =
chrome_browser_state_->GetPrefs()->Get(prefs::kIosUserZoomMultipliers);
EXPECT_EQ(1.1, pref->FindDoublePath(google_pref_key));
EXPECT_EQ(0.9, pref->FindDoublePath(google_amp_pref_key));
}
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