Commit 7e6a9d4e authored by Alice Boxhall's avatar Alice Boxhall Committed by Commit Bot

Revert "[OsSettingsDeepLinking] Add DeepLinkingBehavior & Deep link OSResetPage"

This reverts commit 38a4c6af.

Reason for revert: Newly added test is failing: https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/19641
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8874475110456145024/+/steps/browser_tests/0/logs/Deterministic_failure:_OSSettingsResetPageV3Test.All__status_FAILURE_/0

Original change's description:
> [OsSettingsDeepLinking] Add DeepLinkingBehavior & Deep link OSResetPage
> 
> Added the Polymer Behavior to fetch the settingId parameter from the
> URL and then focus on elements containing that settingId. Imported in
> the OS Reset Page.
> 
> Bug: 1084154
> 
> Change-Id: I52bb73227447d6293a7b31402f7cc5a29e045309
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296008
> Commit-Queue: Daniel Classon <dclasson@google.com>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#789709}

TBR=khorimoto@chromium.org,tjohnsonkanu@google.com,dclasson@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1084154
Change-Id: I1dcf9ed2ea428e69918db46df581c4614a19a40f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2306122Reviewed-by: default avatarAlice Boxhall <aboxhall@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789860}
parent 49774816
......@@ -146,7 +146,6 @@ if (optimize_webui) {
group("closure_compile") {
deps = [
":deep_linking_behavior",
":metrics_recorder",
":os_page_visibility",
":os_route",
......@@ -186,15 +185,6 @@ group("closure_compile") {
]
}
js_library("deep_linking_behavior") {
deps = [
"..:router",
"//chrome/browser/ui/webui/settings/chromeos/constants:mojom_js_library_for_compile",
"//ui/webui/resources/js:assert",
"//ui/webui/resources/js:load_time_data",
]
}
js_library("os_page_visibility") {
deps = [
"//ui/webui/resources/js:cr",
......@@ -298,17 +288,6 @@ js_type_check("closure_compile_local_module") {
]
}
js_library("deep_linking_behavior.m") {
sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/deep_linking_behavior.m.js" ]
deps = [
"..:router.m",
"//chrome/browser/ui/webui/settings/chromeos/constants:mojom_js_library_for_compile",
"//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:load_time_data.m",
]
extra_deps = [ ":modulize" ]
}
js_library("metrics_recorder.m") {
sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/metrics_recorder.m.js" ]
deps = [ "//chrome/browser/ui/webui/settings/chromeos/search:mojo_bindings_js_library_for_compile" ]
......@@ -455,7 +434,6 @@ js_modulizer("modulize") {
"route_origin_behavior.js",
"search_handler.js",
"os_route.js",
"deep_linking_behavior.js",
]
namespace_rewrites = os_settings_namespace_rewrites
}
<link rel="import" href="../router.html">
<link rel="import" href="chrome://resources/html/load_time_data.html">
<script src="chrome://os-settings/constants/setting.mojom-lite.js"></script>
<script src="deep_linking_behavior.js"></script>
\ No newline at end of file
// Copyright 2020 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.
/**
* @fileoverview Polymer behavior for scrolling/focusing/indicating
* setting elements with deep links.
*/
// clang-format off
// #import 'chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.js'
// #import '../constants/setting.mojom-lite.js';
// #import {afterNextRender, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
// #import {assert} from 'chrome://resources/js/assert.m.js';
// #import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
// #import {Router} from '../router.m.js';
// clang-format on
const kDeepLinkSettingId = 'settingId';
const kDeepLinkFocusId = 'deep-link-focus-id';
/** @polymerBehavior */
/* #export */ const DeepLinkingBehavior = {
properties: {
/**
* An object whose values are the kSetting mojom values which can be used to
* define deep link IDs on HTML elems.
* @type {!Object}
*/
Setting: {
type: Object,
value: chromeos.settings.mojom.Setting,
},
},
/**
* Retrieves the settingId saved in the url's query parameter. Returns null if
* deep linking is disabled or if no settingId is found.
* @return {?chromeos.settings.mojom.Setting}
*/
getDeepLinkSettingId() {
if (!loadTimeData.getBoolean('isDeepLinkingEnabled')) {
return null;
}
const settingIdStr = settings.Router.getInstance().getQueryParameters().get(
kDeepLinkSettingId);
const settingIdNum = Number(settingIdStr);
if (isNaN(settingIdNum)) {
return null;
}
return /** @type {!chromeos.settings.mojom.Setting} */ (settingIdNum);
},
/**
* Focuses the deep linked element referred to by |settindId|.
* @param {chromeos.settings.mojom.Setting} settingId
*/
showDeepLink(settingId) {
assert(loadTimeData.getBoolean('isDeepLinkingEnabled'));
const elToFocus = this.$$(`[${kDeepLinkFocusId}~="${settingId}"]`);
Polymer.RenderStatus.afterNextRender(this, () => {
elToFocus.focus();
});
}
};
......@@ -26,11 +26,9 @@ js_library("os_powerwash_dialog") {
js_library("os_reset_page") {
deps = [
"..:deep_linking_behavior",
"..:os_route",
"../..:router",
"//ui/webui/resources/js:assert",
"//ui/webui/resources/js:cr",
"//ui/webui/resources/js:load_time_data",
"//ui/webui/resources/js/cr/ui:focus_without_ink",
]
}
......@@ -73,11 +71,9 @@ js_library("os_reset_browser_proxy.m") {
js_library("os_reset_page.m") {
sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/os_reset_page/os_reset_page.m.js" ]
deps = [
"..:deep_linking_behavior.m",
"..:os_route.m",
"../..:router.m",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:load_time_data.m",
"//ui/webui/resources/js/cr/ui:focus_without_ink.m",
]
extra_deps = [ ":os_reset_page_module" ]
......
......@@ -3,10 +3,7 @@
<link rel="import" href="chrome://resources/html/assert.html">
<link rel="import" href="chrome://resources/html/cr/ui/focus_without_ink.html">
<link rel="import" href="os_powerwash_dialog.html">
<link rel="import" href="../deep_linking_behavior.html">
<link rel="import" href="../os_route.html">
<link rel="import" href="../../i18n_setup.html">
<link rel="import" href="../../router.html">
<dom-module id="os-settings-reset-page">
<template>
......@@ -25,8 +22,7 @@
on-click="onShowPowerwashDialog_"
aria-labelledby="title"
aria-describedby="secondaryText"
aria-roledescription="$i18n{powerwashButtonRoleDescription}"
deep-link-focus-id$="[[Setting.kPowerwash]]">
aria-roledescription="$i18n{powerwashButtonRoleDescription}">
$i18n{powerwashButton}
</cr-button>
</div>
......
......@@ -10,7 +10,6 @@
Polymer({
is: 'os-settings-reset-page',
behaviors: [DeepLinkingBehavior, settings.RouteObserverBehavior],
properties: {
/** @private */
......@@ -32,22 +31,4 @@ Polymer({
this.showPowerwashDialog_ = false;
cr.ui.focusWithoutInk(assert(this.$.powerwash));
},
/**
* settings.RouteObserverBehavior
* @param {!settings.Route} newRoute
* @param {!settings.Route} oldRoute
* @protected
*/
currentRouteChanged(newRoute, oldRoute) {
// Does not apply to this page.
if (newRoute != settings.routes.OS_RESET) {
return;
}
const settingId = this.getDeepLinkSettingId();
if (settingId === chromeos.settings.mojom.Setting.kPowerwash) {
this.showDeepLink(settingId);
}
},
});
......@@ -32,30 +32,29 @@ os_settings_namespace_rewrites =
"settings.WallpaperBrowserProxy|WallpaperBrowserProxy",
]
os_settings_auto_imports = settings_auto_imports +
cr_components_chromeos_auto_imports +
cr_elements_chromeos_auto_imports + [
"chrome/browser/resources/settings/chromeos/ambient_mode_page/constants.html|AmbientModeTopicSource,AmbientModeSettings",
"chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_browser_proxy.html|AmbientModeBrowserProxy,AmbientModeBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/deep_linking_behavior.html|DeepLinkingBehavior",
"chrome/browser/resources/settings/chromeos/metrics_recorder.html|recordSettingChange",
"chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_constants.html|MultiDeviceSettingsMode,MultiDeviceFeature,MultiDeviceFeatureState,MultiDevicePageContentData,SmartLockSignInEnabledState",
"chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_feature_behavior.html|MultiDeviceFeatureBehavior",
"chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_browser_proxy.html|MultiDeviceBrowserProxy,MultiDeviceBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/os_people_page/kerberos_accounts_browser_proxy.html|KerberosAccount,KerberosAccountsBrowserProxyImpl,KerberosAccountsBrowserProxy,KerberosErrorType,KerberosConfigErrorCode,ValidateKerberosConfigResult",
"chrome/browser/resources/settings/chromeos/os_people_page/os_sync_browser_proxy.html|OsSyncBrowserProxy,OsSyncBrowserProxyImpl,OsSyncPrefs",
"chrome/browser/resources/settings/chromeos/os_reset_page/os_reset_browser_proxy.html|OsResetBrowserProxy,OsResetBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/os_route.html|routes",
"chrome/browser/resources/settings/chromeos/os_settings_routes.html|OsSettingsRoutes",
"chrome/browser/resources/settings/chromeos/personalization_page/change_picture_browser_proxy.html|ChangePictureBrowserProxy,ChangePictureBrowserProxyImpl,DefaultImage",
"chrome/browser/resources/settings/chromeos/personalization_page/wallpaper_browser_proxy.html|WallpaperBrowserProxy,WallpaperBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/route_origin_behavior.html|RouteOriginBehaviorImpl,RouteOriginBehavior",
"chrome/browser/resources/settings/lifetime_browser_proxy.html|LifetimeBrowserProxy,LifetimeBrowserProxyImpl",
"chrome/browser/resources/settings/people_page/account_manager_browser_proxy.html|AccountManagerBrowserProxy,AccountManagerBrowserProxyImpl",
"chrome/browser/resources/settings/route.html|routes",
"chrome/browser/resources/settings/router.html|Router,Route,RouteObserverBehavior",
"ui/webui/resources/html/polymer.html|Polymer,html,flush",
"ui/webui/resources/html/icon.html|getImage",
]
os_settings_auto_imports =
settings_auto_imports + cr_components_chromeos_auto_imports +
cr_elements_chromeos_auto_imports + [
"chrome/browser/resources/settings/chromeos/ambient_mode_page/constants.html|AmbientModeTopicSource,AmbientModeSettings",
"chrome/browser/resources/settings/chromeos/ambient_mode_page/ambient_mode_browser_proxy.html|AmbientModeBrowserProxy,AmbientModeBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/metrics_recorder.html|recordSettingChange",
"chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_constants.html|MultiDeviceSettingsMode,MultiDeviceFeature,MultiDeviceFeatureState,MultiDevicePageContentData,SmartLockSignInEnabledState",
"chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_feature_behavior.html|MultiDeviceFeatureBehavior",
"chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_browser_proxy.html|MultiDeviceBrowserProxy,MultiDeviceBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/os_people_page/kerberos_accounts_browser_proxy.html|KerberosAccount,KerberosAccountsBrowserProxyImpl,KerberosAccountsBrowserProxy,KerberosErrorType,KerberosConfigErrorCode,ValidateKerberosConfigResult",
"chrome/browser/resources/settings/chromeos/os_people_page/os_sync_browser_proxy.html|OsSyncBrowserProxy,OsSyncBrowserProxyImpl,OsSyncPrefs",
"chrome/browser/resources/settings/chromeos/os_reset_page/os_reset_browser_proxy.html|OsResetBrowserProxy,OsResetBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/os_route.html|routes",
"chrome/browser/resources/settings/chromeos/os_settings_routes.html|OsSettingsRoutes",
"chrome/browser/resources/settings/chromeos/personalization_page/change_picture_browser_proxy.html|ChangePictureBrowserProxy,ChangePictureBrowserProxyImpl,DefaultImage",
"chrome/browser/resources/settings/chromeos/personalization_page/wallpaper_browser_proxy.html|WallpaperBrowserProxy,WallpaperBrowserProxyImpl",
"chrome/browser/resources/settings/chromeos/route_origin_behavior.html|RouteOriginBehaviorImpl,RouteOriginBehavior",
"chrome/browser/resources/settings/lifetime_browser_proxy.html|LifetimeBrowserProxy,LifetimeBrowserProxyImpl",
"chrome/browser/resources/settings/people_page/account_manager_browser_proxy.html|AccountManagerBrowserProxy,AccountManagerBrowserProxyImpl",
"chrome/browser/resources/settings/route.html|routes",
"chrome/browser/resources/settings/router.html|Router,Route,RouteObserverBehavior",
"ui/webui/resources/html/polymer.html|Polymer,html,flush",
"ui/webui/resources/html/icon.html|getImage",
]
os_settings_migrated_imports = settings_migrated_imports
......@@ -117,11 +117,6 @@
use_base_dir="false"
compress="false"
type="BINDATA" />
<include name="IDR_OS_SETTINGS_DEEP_LINKING_BEHAVIOR_M_JS"
file="${root_gen_dir}/chrome/browser/resources/settings/chromeos/deep_linking_behavior.m.js"
use_base_dir="false"
compress="false"
type="BINDATA" />
<include name="IDR_OS_SETTINGS_LOCALIZED_LINK_M_JS"
file="${root_gen_dir}/chrome/browser/resources/settings/chromeos/localized_link/localized_link.m.js"
use_base_dir="false"
......
......@@ -510,12 +510,6 @@
<structure name="IDR_OS_SETTINGS_CONTROLS_TOGGLE_BUTTON_JS"
file="controls/settings_toggle_button.js"
compress="false" type="chrome_html" />
<structure name="IDR_OS_SETTINGS_DEEP_LINKING_BEHAVIOR_HTML"
file="chromeos/deep_linking_behavior.html"
compress="false" type="chrome_html" />
<structure name="IDR_OS_SETTINGS_DEEP_LINKING_BEHAVIOR_JS"
file="chromeos/deep_linking_behavior.js"
compress="false" type="chrome_html" />
<structure name="IDR_OS_SETTINGS_DEVICE_BROWSER_PROXY_HTML"
file="chromeos/device_page/device_page_browser_proxy.html"
compress="false" type="chrome_html" />
......
......@@ -2,23 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// clang-format off
// #import {TestLifetimeBrowserProxy} from './test_os_lifetime_browser_proxy.m.js';
// #import {LifetimeBrowserProxy, LifetimeBrowserProxyImpl, OsResetBrowserProxyImpl} from 'chrome://os-settings/chromeos/os_settings.js';
// #import {TestOsResetBrowserProxy} from './test_os_reset_browser_proxy.m.js';
// #import {assertEquals, assertFalse, assertNotEquals, assertTrue} from '../../chai_assert.js';
// #import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
// #import {getDeepActiveElement} from 'chrome://resources/js/util.m.js';
// clang-format on
cr.define('settings_reset_page', function() {
/** @enum {string} */
const TestNames = {
PowerwashDialogAction: 'PowerwashDialogAction',
PowerwashDialogOpenClose: 'PowerwashDialogOpenClose',
PowerwashFocusDeepLink: 'PowerwashFocusDeepLink',
PowerwashFocusDeepLinkNoFlag: 'PowerwashFocusDeepLinkNoFlag',
PowerwashFocusDeepLinkWrongId: 'PowerwashFocusDeepLinkWrongId',
};
suite('DialogTests', function() {
......@@ -44,7 +38,6 @@ cr.define('settings_reset_page', function() {
});
teardown(function() {
settings.Router.getInstance().resetRouteForTesting();
resetPage.remove();
});
......@@ -76,23 +69,6 @@ cr.define('settings_reset_page', function() {
]);
}
/**
* Navigates to the deep link provided by |settingId| and returns true if
* the focused element is |deepLinkElement|.
* @param {!Element} deepLinkElement
* @param {!string} settingId
* @returns {!boolean}
*/
async function isDeepLinkFocusedForSettingId(deepLinkElement, settingId) {
const params = new URLSearchParams;
params.append('settingId', settingId);
settings.Router.getInstance().navigateTo(
settings.routes.OS_RESET, params);
await test_util.waitAfterNextRender(deepLinkElement);
return deepLinkElement === getDeepActiveElement();
}
// Tests that the powerwash dialog opens and closes correctly, and
// that chrome.send calls are propagated as expected.
test(TestNames.PowerwashDialogOpenClose, function() {
......@@ -115,36 +91,6 @@ cr.define('settings_reset_page', function() {
await lifetimeBrowserProxy.whenCalled('factoryReset');
assertFalse(requestTpmFirmwareUpdate);
});
// Tests that when the route changes to one containing a deep link to
// powerwash, powerwash is focused.
test(TestNames.PowerwashFocusDeepLink, async () => {
loadTimeData.overrideValues({isDeepLinkingEnabled: true});
assertTrue(loadTimeData.getBoolean('isDeepLinkingEnabled'));
assertTrue(
await isDeepLinkFocusedForSettingId(resetPage.$.powerwash, '1600'),
'Powerwash should be focused for settingId=1600.');
});
// Tests that when the deep linking flag is disabled, no focusing of deep
// links occurs.
test(TestNames.PowerwashFocusDeepLinkNoFlag, async () => {
loadTimeData.overrideValues({isDeepLinkingEnabled: false});
assertFalse(loadTimeData.getBoolean('isDeepLinkingEnabled'));
assertFalse(
await isDeepLinkFocusedForSettingId(resetPage.$.powerwash, '1600'),
'Powerwash should not be focused with flag disabled.');
});
// Tests that when the route changes to one containing a deep link not equal
// to powerwash, no focusing of powerwash occurs.
test(TestNames.PowerwashFocusDeepLinkWrongId, async () => {
loadTimeData.overrideValues({isDeepLinkingEnabled: true});
assertTrue(loadTimeData.getBoolean('isDeepLinkingEnabled'));
assertFalse(
await isDeepLinkFocusedForSettingId(resetPage.$.powerwash, '1234'),
'Powerwash should not be focused for settingId=1234.');
});
});
// #cr_define_end
......
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