Commit 0f6220ee authored by Xing Liu's avatar Xing Liu Committed by Chromium LUCI CQ

Read later: Disable the reading list entry point for invalid URL.

This matches the behavior when the URL is not supported by download.

Bug: 1157808
Change-Id: I5300adefc2d4f8e90c2345d0aacb38514ba55d11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2616481
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841328}
parent 366e9dc9
...@@ -1193,6 +1193,7 @@ chrome_java_sources = [ ...@@ -1193,6 +1193,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/rappor/RapporServiceBridge.java", "java/src/org/chromium/chrome/browser/rappor/RapporServiceBridge.java",
"java/src/org/chromium/chrome/browser/read_later/ReadLaterIPHController.java", "java/src/org/chromium/chrome/browser/read_later/ReadLaterIPHController.java",
"java/src/org/chromium/chrome/browser/read_later/ReadingListBridge.java", "java/src/org/chromium/chrome/browser/read_later/ReadingListBridge.java",
"java/src/org/chromium/chrome/browser/read_later/ReadingListUtils.java",
"java/src/org/chromium/chrome/browser/reengagement/ReengagementNotificationController.java", "java/src/org/chromium/chrome/browser/reengagement/ReengagementNotificationController.java",
"java/src/org/chromium/chrome/browser/resources/ResourceMapper.java", "java/src/org/chromium/chrome/browser/resources/ResourceMapper.java",
"java/src/org/chromium/chrome/browser/rlz/RevenueStats.java", "java/src/org/chromium/chrome/browser/rlz/RevenueStats.java",
......
...@@ -198,6 +198,7 @@ chrome_junit_test_java_sources = [ ...@@ -198,6 +198,7 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/payments/handler/toolbar/PaymentHandlerToolbarMediatorTest.java", "junit/src/org/chromium/chrome/browser/payments/handler/toolbar/PaymentHandlerToolbarMediatorTest.java",
"junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java", "junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java",
"junit/src/org/chromium/chrome/browser/privacy/settings/PrivacyPreferencesManagerImplTest.java", "junit/src/org/chromium/chrome/browser/privacy/settings/PrivacyPreferencesManagerImplTest.java",
"junit/src/org/chromium/chrome/browser/read_later/ReadingListUtilsUnitTest.java",
"junit/src/org/chromium/chrome/browser/reengagement/ReengagementNotificationControllerTest.java", "junit/src/org/chromium/chrome/browser/reengagement/ReengagementNotificationControllerTest.java",
"junit/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingReferringAppBridgeTest.java", "junit/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingReferringAppBridgeTest.java",
"junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java", "junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java",
......
...@@ -49,6 +49,7 @@ import org.chromium.chrome.browser.incognito.IncognitoUtils; ...@@ -49,6 +49,7 @@ import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.multiwindow.MultiWindowModeStateDispatcher; import org.chromium.chrome.browser.multiwindow.MultiWindowModeStateDispatcher;
import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper; import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.read_later.ReadingListUtils;
import org.chromium.chrome.browser.share.ShareHelper; import org.chromium.chrome.browser.share.ShareHelper;
import org.chromium.chrome.browser.share.ShareUtils; import org.chromium.chrome.browser.share.ShareUtils;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
...@@ -448,6 +449,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate ...@@ -448,6 +449,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
addToMenuItem.getSubMenu().findItem(R.id.add_to_reading_list_menu_id); addToMenuItem.getSubMenu().findItem(R.id.add_to_reading_list_menu_id);
addToReadingListMenuItem.setVisible( addToReadingListMenuItem.setVisible(
CachedFeatureFlags.isEnabled(ChromeFeatureList.READ_LATER)); CachedFeatureFlags.isEnabled(ChromeFeatureList.READ_LATER));
addToReadingListMenuItem.setEnabled(ReadingListUtils.isReadingListSupported(url));
MenuItem addToDownloadsMenuItem = MenuItem addToDownloadsMenuItem =
addToMenuItem.getSubMenu().findItem(R.id.add_to_downloads_menu_id); addToMenuItem.getSubMenu().findItem(R.id.add_to_downloads_menu_id);
......
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.read_later;
import android.text.TextUtils;
import org.chromium.components.embedder_support.util.UrlConstants;
/**
* Utility functions for reading list feature.
*/
public final class ReadingListUtils {
/**
* @return Whether the URL can be added as reading list article.
*/
public static boolean isReadingListSupported(String url) {
if (TextUtils.isEmpty(url)) return false;
// This should match ReadingListModel::IsUrlSupported(), having a separate function since
// the UI may not load native library.
return url.startsWith(UrlConstants.HTTP_URL_PREFIX)
|| url.startsWith(UrlConstants.HTTPS_URL_PREFIX);
}
}
...@@ -11,6 +11,7 @@ import static org.mockito.Mockito.when; ...@@ -11,6 +11,7 @@ import static org.mockito.Mockito.when;
import android.content.Context; import android.content.Context;
import android.view.Menu; import android.view.Menu;
import android.view.MenuItem;
import android.view.SubMenu; import android.view.SubMenu;
import android.view.View; import android.view.View;
import android.widget.PopupMenu; import android.widget.PopupMenu;
...@@ -70,7 +71,10 @@ import org.chromium.net.ConnectionType; ...@@ -70,7 +71,10 @@ import org.chromium.net.ConnectionType;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set;
/** /**
* Unit tests for {@link AppMenuPropertiesDelegateImpl}. * Unit tests for {@link AppMenuPropertiesDelegateImpl}.
...@@ -575,6 +579,8 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -575,6 +579,8 @@ public class AppMenuPropertiesDelegateUnitTest {
assertMenuItemsAreEqual(menu, expectedItems); assertMenuItemsAreEqual(menu, expectedItems);
assertActionBarItemsAreEqual(menu, expectedActionBarItems); assertActionBarItemsAreEqual(menu, expectedActionBarItems);
assertAddToItemsAreEqual(menu, expectedAddToItems); assertAddToItemsAreEqual(menu, expectedAddToItems);
assertAddToItemsEnableState(
menu, new HashSet<>(Arrays.asList(R.id.add_to_reading_list_menu_id)), null);
} }
@Test @Test
...@@ -915,6 +921,20 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -915,6 +921,20 @@ public class AppMenuPropertiesDelegateUnitTest {
Matchers.containsInAnyOrder(expectedItems)); Matchers.containsInAnyOrder(expectedItems));
} }
private void assertAddToItemsEnableState(
Menu menu, Set<Integer> enabledItems, Set<Integer> disabledItems) {
SubMenu addToSubMenu = menu.findItem(R.id.add_to_menu_id).getSubMenu();
for (int i = 0; i < addToSubMenu.size(); i++) {
MenuItem item = addToSubMenu.getItem(i);
if (enabledItems != null && enabledItems.contains(item.getItemId())) {
Assert.assertTrue(item.isEnabled());
}
if (disabledItems != null && disabledItems.contains(item.getItemId())) {
Assert.assertFalse(item.isEnabled());
}
}
}
private String getMenuTitles(Menu menu) { private String getMenuTitles(Menu menu) {
StringBuilder items = new StringBuilder(); StringBuilder items = new StringBuilder();
for (int i = 0; i < menu.size(); i++) { for (int i = 0; i < menu.size(); i++) {
......
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.read_later;
import android.support.test.filters.SmallTest;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.annotation.Config;
import org.chromium.base.test.BaseRobolectricTestRunner;
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE)
/**
* Unit test for {@link ReadingListUtils}.
*/
public class ReadingListUtilsUnitTest {
@Test
@SmallTest
public void testIsReadingListSupport() {
Assert.assertFalse(ReadingListUtils.isReadingListSupported(null));
Assert.assertFalse(ReadingListUtils.isReadingListSupported(""));
Assert.assertFalse(ReadingListUtils.isReadingListSupported("chrome://flags"));
Assert.assertTrue(ReadingListUtils.isReadingListSupported("http://www.example.com"));
Assert.assertTrue(ReadingListUtils.isReadingListSupported("https://www.example.com"));
}
}
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