Commit 0331f453 authored by Robert Ogden's avatar Robert Ogden Committed by Commit Bot

Bug fixes for Previews Omnibox UI

There's a few bug fixes in this CL but it seems ok to lump them
together since each bug is in a separate file.

CustomTabActivity.java - don't show AMP UI on a Preview

ToolbarManager.java - also update Previews UI on commit

ToolbarModel.java - explicity remove the scheme from Previews

Bug: 896478
Change-Id: Id8353a8787481bf4fcbfa322994d8b0ec998d6df
Reviewed-on: https://chromium-review.googlesource.com/c/1287289
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600779}
parent f4fe17ed
...@@ -1556,6 +1556,10 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent ...@@ -1556,6 +1556,10 @@ public class CustomTabActivity extends ChromeActivity<CustomTabActivityComponent
return false; return false;
} }
if (mMainTab != null && mMainTab.isPreview()) {
return false;
}
String publisherUrlPackage = mConnection.getTrustedCdnPublisherUrlPackage(); String publisherUrlPackage = mConnection.getTrustedCdnPublisherUrlPackage();
return publisherUrlPackage != null return publisherUrlPackage != null
&& publisherUrlPackage.equals(mConnection.getClientPackageNameForSession(mSession)); && publisherUrlPackage.equals(mConnection.getClientPackageNameForSession(mSession));
......
...@@ -287,23 +287,6 @@ public class OfflinePageUtils { ...@@ -287,23 +287,6 @@ public class OfflinePageUtils {
|| webContents.isDestroyed() || webContents.isIncognito(); || webContents.isDestroyed() || webContents.isIncognito();
} }
/**
* Strips scheme from the original URL of the offline page. This is meant to be used by UI.
* @param onlineUrl an online URL to from which the scheme is removed
* @return onlineUrl without the scheme
*/
public static String stripSchemeFromOnlineUrl(String onlineUrl) {
onlineUrl = onlineUrl.trim();
// Offline pages are only saved for https:// and http:// schemes.
if (onlineUrl.startsWith(UrlConstants.HTTPS_URL_PREFIX)) {
return onlineUrl.substring(8);
} else if (onlineUrl.startsWith(UrlConstants.HTTP_URL_PREFIX)) {
return onlineUrl.substring(7);
} else {
return onlineUrl;
}
}
/** /**
* Shows the snackbar for the current tab to provide offline specific information if needed. * Shows the snackbar for the current tab to provide offline specific information if needed.
* @param tab The current tab. * @param tab The current tab.
......
...@@ -207,7 +207,7 @@ public class PageInfoController ...@@ -207,7 +207,7 @@ public class PageInfoController
String displayUrl = UrlFormatter.formatUrlForCopy(mFullUrl); String displayUrl = UrlFormatter.formatUrlForCopy(mFullUrl);
if (isShowingOfflinePage()) { if (isShowingOfflinePage()) {
displayUrl = OfflinePageUtils.stripSchemeFromOnlineUrl(mFullUrl); displayUrl = UrlUtilities.stripScheme(mFullUrl);
} }
SpannableStringBuilder displayUrlBuilder = new SpannableStringBuilder(displayUrl); SpannableStringBuilder displayUrlBuilder = new SpannableStringBuilder(displayUrl);
OmniboxUrlEmphasizer.emphasizeUrl(displayUrlBuilder, mContext.getResources(), OmniboxUrlEmphasizer.emphasizeUrl(displayUrlBuilder, mContext.getResources(),
......
...@@ -410,10 +410,12 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe ...@@ -410,10 +410,12 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
return; return;
} }
// TODO(crbug.com/896476): Remove this.
if (tab.isPreview()) { if (tab.isPreview()) {
// Some previews are not fully decided until the page finishes loading. If this // Some previews (like Client LoFi) are not fully decided until the page
// is a preview, update the security icon which will also update the verbose // finishes loading. If this is a preview, update the security icon which will
// status view to make sure the "Lite" badge is displayed. // also update the verbose status view to make sure the "Lite" badge is
// displayed.
mLocationBar.updateSecurityIcon(); mLocationBar.updateSecurityIcon();
} }
...@@ -552,6 +554,13 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe ...@@ -552,6 +554,13 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
mToolbar.onNavigatedToDifferentPage(); mToolbar.onNavigatedToDifferentPage();
} }
if (hasCommitted && tab.isPreview()) {
// Some previews are not fully decided until the page commits. If this
// is a preview, update the security icon which will also update the verbose
// status view to make sure the "Lite" badge is displayed.
mLocationBar.updateSecurityIcon();
}
// If the load failed due to a different navigation, there is no need to reset the // If the load failed due to a different navigation, there is no need to reset the
// location bar animations. // location bar animations.
if (errorCode != 0 && isInMainFrame && !hasPendingNonNtpNavigation(tab)) { if (errorCode != 0 && isInMainFrame && !hasPendingNonNtpNavigation(tab)) {
......
...@@ -166,9 +166,14 @@ public class ToolbarModel implements ToolbarDataProvider { ...@@ -166,9 +166,14 @@ public class ToolbarModel implements ToolbarDataProvider {
return buildUrlBarData(url, formattedUrl); return buildUrlBarData(url, formattedUrl);
} }
if (isPreview()) {
// Strip the scheme from the original URL for the Previews UI.
return buildUrlBarData(url, UrlUtilities.stripScheme(url));
}
if (isOfflinePage()) { if (isOfflinePage()) {
String originalUrl = mTab.getOriginalUrl(); String originalUrl = mTab.getOriginalUrl();
formattedUrl = OfflinePageUtils.stripSchemeFromOnlineUrl( formattedUrl = UrlUtilities.stripScheme(
DomDistillerTabUtils.getFormattedUrlFromOriginalDistillerUrl(originalUrl)); DomDistillerTabUtils.getFormattedUrlFromOriginalDistillerUrl(originalUrl));
// Clear the editing text for untrusted offline pages. // Clear the editing text for untrusted offline pages.
......
...@@ -344,6 +344,20 @@ public class UrlUtilities { ...@@ -344,6 +344,20 @@ public class UrlUtilities {
+ ((parsed.getPort() != -1) ? (":" + parsed.getPort()) : ""); + ((parsed.getPort() != -1) ? (":" + parsed.getPort()) : "");
} }
/**
* @param url An HTTP or HTTPS URL.
* @return The URL without the scheme.
*/
public static String stripScheme(String url) {
String noScheme = url.trim();
if (noScheme.startsWith(UrlConstants.HTTPS_URL_PREFIX)) {
noScheme = noScheme.substring(8);
} else if (noScheme.startsWith(UrlConstants.HTTP_URL_PREFIX)) {
noScheme = noScheme.substring(7);
}
return noScheme;
}
private static native boolean nativeIsDownloadable(String url); private static native boolean nativeIsDownloadable(String url);
private static native boolean nativeIsValidForIntentFallbackNavigation(String url); private static native boolean nativeIsValidForIntentFallbackNavigation(String url);
private static native boolean nativeIsAcceptedScheme(String url); private static native boolean nativeIsAcceptedScheme(String url);
......
...@@ -106,27 +106,6 @@ public class OfflinePageUtilsUnitTest { ...@@ -106,27 +106,6 @@ public class OfflinePageUtilsUnitTest {
assertEquals(56789L, OfflinePageUtils.getTotalSpaceInBytes()); assertEquals(56789L, OfflinePageUtils.getTotalSpaceInBytes());
} }
@Test
@Feature({"OfflinePages"})
public void testStripSchemeFromOnlineUrl() {
// Only scheme gets stripped.
assertEquals("cs.chromium.org",
OfflinePageUtils.stripSchemeFromOnlineUrl("https://cs.chromium.org"));
assertEquals("cs.chromium.org",
OfflinePageUtils.stripSchemeFromOnlineUrl("http://cs.chromium.org"));
// If there is no scheme, nothing changes.
assertEquals(
"cs.chromium.org", OfflinePageUtils.stripSchemeFromOnlineUrl("cs.chromium.org"));
// Path is not touched/changed.
String urlWithPath = "code.google.com/p/chromium/codesearch#search"
+ "/&q=offlinepageutils&sq=package:chromium&type=cs";
assertEquals(
urlWithPath, OfflinePageUtils.stripSchemeFromOnlineUrl("https://" + urlWithPath));
// Beginning and ending spaces get trimmed.
assertEquals("cs.chromium.org",
OfflinePageUtils.stripSchemeFromOnlineUrl(" https://cs.chromium.org "));
}
@Test @Test
@Feature({"OfflinePages"}) @Feature({"OfflinePages"})
public void testSaveBookmarkOffline() { public void testSaveBookmarkOffline() {
......
...@@ -124,4 +124,20 @@ public class UrlUtilitiesUnitTest { ...@@ -124,4 +124,20 @@ public class UrlUtilitiesUnitTest {
Assert.assertEquals("http://localhost", UrlUtilities.stripPath("http://localhost/")); Assert.assertEquals("http://localhost", UrlUtilities.stripPath("http://localhost/"));
Assert.assertEquals("http://", UrlUtilities.stripPath("http:")); Assert.assertEquals("http://", UrlUtilities.stripPath("http:"));
} }
@Test
public void testStripScheme() {
// Only scheme gets stripped.
Assert.assertEquals("cs.chromium.org", UrlUtilities.stripScheme("https://cs.chromium.org"));
Assert.assertEquals("cs.chromium.org", UrlUtilities.stripScheme("http://cs.chromium.org"));
// If there is no scheme, nothing changes.
Assert.assertEquals("cs.chromium.org", UrlUtilities.stripScheme("cs.chromium.org"));
// Path is not touched/changed.
String urlWithPath = "code.google.com/p/chromium/codesearch#search"
+ "/&q=testStripScheme&sq=package:chromium&type=cs";
Assert.assertEquals(urlWithPath, UrlUtilities.stripScheme("https://" + urlWithPath));
// Beginning and ending spaces get trimmed.
Assert.assertEquals(
"cs.chromium.org", UrlUtilities.stripScheme(" https://cs.chromium.org "));
}
} }
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