Skip to content

Commit

Permalink
[CrOS Cellular] Remove extraneous showConfigurableSections_ checks.
Browse files Browse the repository at this point in the history 
Remove extra showConfigurableSections_ checks in internet-detail-page
and internet-detail-dialog that would hide Configure, Forget, etc.
buttons.
Fix internet-detail-dialog error where no managedProperties_ would
cause showConnect_ and showDisconnect_ to error. This bug was not
visible before this change as these methods were behind
showConfigurableSections_ which always checks for defined
managedProperties_.

(cherry picked from commit 2460b44)

Fixed: 1198625
Change-Id: Ia64201a99480c63399d2a99d1f5ae6a6f03d8bb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824587
Commit-Queue: Gordon Seto <gordonseto@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#872184}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2827196
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Kyle Horimoto <khorimoto@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4472@{#88}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}
  • Loading branch information
Gordon Seto authored and Chromium LUCI CQ committed Apr 15, 2021
1 parent 898329c commit 7dac310
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 118 deletions.
24 changes: 11 additions & 13 deletions chrome/browser/resources/chromeos/internet_detail_dialog/internet_detail_dialog.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,17 @@
connected$="[[isConnectedState_(managedProperties_)]]">
[[getStateText_(managedProperties_)]]
</div>
<template is="dom-if" if="[[showConfigurableSections_]]" restamp>
<cr-button on-click="onForgetTap_"
hidden$="[[!showForget_(managedProperties_)]]"
disabled="[[disabled_]]">
$i18n{networkButtonForget}
</cr-button>
<cr-button id="connectDisconnect"
class="action-button" on-click="onConnectDisconnectClick_"
hidden$="[[!showConnectDisconnect_(managedProperties_)]]"
disabled="[[!enableConnectDisconnect_(managedProperties_, disabled_)]]">
[[getConnectDisconnectText_(managedProperties_)]]
</cr-button>
</template>
<cr-button on-click="onForgetTap_"
hidden$="[[!showForget_(managedProperties_)]]"
disabled="[[disabled_]]">
$i18n{networkButtonForget}
</cr-button>
<cr-button id="connectDisconnect"
class="action-button" on-click="onConnectDisconnectClick_"
hidden$="[[!showConnectDisconnect_(managedProperties_)]]"
disabled="[[!enableConnectDisconnect_(managedProperties_, disabled_)]]">
[[getConnectDisconnectText_(managedProperties_)]]
</cr-button>
</div>

<template is="dom-if" if="[[showConfigurableSections_]]" restamp>
Expand Down
15 changes: 12 additions & 3 deletions chrome/browser/resources/chromeos/internet_detail_dialog/internet_detail_dialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,8 @@ Polymer({
},

/**
* @param {!chromeos.networkConfig.mojom.ManagedProperties} managedProperties
* @param {!chromeos.networkConfig.mojom.ManagedProperties|undefined}
* managedProperties
* @return {boolean}
* @private
*/
Expand All @@ -422,11 +423,15 @@ Polymer({
},

/**
* @param {!chromeos.networkConfig.mojom.ManagedProperties} managedProperties
* @param {!chromeos.networkConfig.mojom.ManagedProperties|undefined}
* managedProperties
* @return {boolean}
* @private
*/
showConnect_(managedProperties) {
if (!managedProperties) {
return false;
}
return managedProperties.connectable &&
managedProperties.type !=
chromeos.networkConfig.mojom.NetworkType.kEthernet &&
Expand All @@ -435,11 +440,15 @@ Polymer({
},

/**
* @param {!chromeos.networkConfig.mojom.ManagedProperties} managedProperties
* @param {!chromeos.networkConfig.mojom.ManagedProperties|undefined}
* managedProperties
* @return {boolean}
* @private
*/
showDisconnect_(managedProperties) {
if (!managedProperties) {
return false;
}
return managedProperties.type !=
chromeos.networkConfig.mojom.NetworkType.kEthernet &&
managedProperties.connectionState !=
Expand Down
186 changes: 91 additions & 95 deletions chrome/browser/resources/settings/chromeos/internet_page/internet_detail_page.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,110 +106,106 @@
</cr-policy-indicator>
</template>
</div>
<template is="dom-if" if="[[showConfigurableSections_]]" restamp>
<cr-button id="forgetButton" on-click="onForgetTap_"
hidden$="[[!showForget_(managedProperties_)]]"
disabled="[[disableForget_(managedProperties_,
prefs.vpn_config_allowed, disabled_)]]">
$i18n{networkButtonForget}
</cr-button>
<cr-button id="viewAccountButton"
on-click="onViewAccountTap_"
hidden$="[[!showViewAccount_(managedProperties_)]]"
disabled="[[disabled_]]">
$i18n{networkButtonViewAccount}
</cr-button>
<cr-button id="activateButton"
on-click="onActivateTap_"
hidden$="[[!showActivate_(managedProperties_)]]"
disabled="[[disabled_]]">
$i18n{networkButtonActivate}
</cr-button>
<cr-button id="configureButton" on-click="onConfigureTap_"
hidden$="[[!showConfigure_(managedProperties_, globalPolicy,
managedNetworkAvailable)]]"
disabled="[[disableConfigure_(managedProperties_,
prefs.vpn_config_allowed, disabled_)]]"
deep-link-focus-id$="[[Setting.kConfigureEthernet]]">
$i18n{networkButtonConfigure}
</cr-button>
<!-- Use policy properties from vpn_config_allowed to indicate when that
pref disables buttons in this row. -->
<controlled-button id="connectDisconnect" class="action-button"
on-click="onConnectDisconnectTap_"
hidden$="[[shouldConnectDisconnectButtonBeHidden_(
managedProperties_, globalPolicy, managedNetworkAvailable,
deviceState_)]]"
disabled="[[shouldConnectDisconnectButtonBeDisabled_(
managedProperties_, defaultNetwork, propertiesReceived_,
outOfRange_, globalPolicy, managedNetworkAvailable,
deviceState_, disabled_)]]"
label="[[getConnectDisconnectButtonLabel_(managedProperties_,
globalPolicy,managedNetworkAvailable, deviceState_)]]"
pref="[[getFakeVpnConfigPrefForEnforcement_(managedProperties_,
prefs.vpn_config_allowed)]]"
deep-link-focus-id$="[[Setting.kDisconnectWifiNetwork]]
[[Setting.kDisconnectCellularNetwork]]
[[Setting.kDisconnectTetherNetwork]]">
</controlled-button>
</template>
<cr-button id="forgetButton" on-click="onForgetTap_"
hidden$="[[!showForget_(managedProperties_)]]"
disabled="[[disableForget_(managedProperties_,
prefs.vpn_config_allowed, disabled_)]]">
$i18n{networkButtonForget}
</cr-button>
<cr-button id="viewAccountButton"
on-click="onViewAccountTap_"
hidden$="[[!showViewAccount_(managedProperties_)]]"
disabled="[[disabled_]]">
$i18n{networkButtonViewAccount}
</cr-button>
<cr-button id="activateButton"
on-click="onActivateTap_"
hidden$="[[!showActivate_(managedProperties_)]]"
disabled="[[disabled_]]">
$i18n{networkButtonActivate}
</cr-button>
<cr-button id="configureButton" on-click="onConfigureTap_"
hidden$="[[!showConfigure_(managedProperties_, globalPolicy,
managedNetworkAvailable)]]"
disabled="[[disableConfigure_(managedProperties_,
prefs.vpn_config_allowed, disabled_)]]"
deep-link-focus-id$="[[Setting.kConfigureEthernet]]">
$i18n{networkButtonConfigure}
</cr-button>
<!-- Use policy properties from vpn_config_allowed to indicate when that
pref disables buttons in this row. -->
<controlled-button id="connectDisconnect" class="action-button"
on-click="onConnectDisconnectTap_"
hidden$="[[shouldConnectDisconnectButtonBeHidden_(
managedProperties_, globalPolicy, managedNetworkAvailable,
deviceState_)]]"
disabled="[[shouldConnectDisconnectButtonBeDisabled_(
managedProperties_, defaultNetwork, propertiesReceived_,
outOfRange_, globalPolicy, managedNetworkAvailable,
deviceState_, disabled_)]]"
label="[[getConnectDisconnectButtonLabel_(managedProperties_,
globalPolicy,managedNetworkAvailable, deviceState_)]]"
pref="[[getFakeVpnConfigPrefForEnforcement_(managedProperties_,
prefs.vpn_config_allowed)]]"
deep-link-focus-id$="[[Setting.kDisconnectWifiNetwork]]
[[Setting.kDisconnectCellularNetwork]]
[[Setting.kDisconnectTetherNetwork]]">
</controlled-button>
</div>


<template is="dom-if" if="[[showConfigurableSections_]]" restamp>
<!-- Start of NOTICES section. -->
<!-- If row ordering changes, messagesDividerClass_() must be updated. -->
<template is="dom-if" if="[[isBlockedByPolicy_(managedProperties_,
globalPolicy, managedNetworkAvailable)]]">
<!-- Disabled by policy -->
<div class="settings-box continuation">
<iron-icon class="policy" icon="cr20:domain"></iron-icon>
<div class="settings-box-text">$i18n{networkConnectNotAllowed}</div>
</div>
</template>
<!-- Start of NOTICES section. -->
<!-- If row ordering changes, messagesDividerClass_() must be updated. -->
<template is="dom-if" if="[[isBlockedByPolicy_(managedProperties_,
globalPolicy, managedNetworkAvailable)]]">
<!-- Disabled by policy -->
<div class="settings-box continuation">
<iron-icon class="policy" icon="cr20:domain"></iron-icon>
<div class="settings-box-text">$i18n{networkConnectNotAllowed}</div>
</div>
</template>

<template is="dom-if" if="[[isSecondaryUser_]]">
<!-- Non primary users. -->
<div class$="settings-box single-column
[[messagesDividerClass_('secondary', managedProperties_,
globalPolicy, managedNetworkAvailable,
isSecondaryUser_, isWifiSyncEnabled_)]]">
<div class="layout horizontal center">
<iron-icon class="policy" icon="cr:group"></iron-icon>
<div class="settings-box-text">
[[i18n('networkPrimaryUserControlled', primaryUserEmail_)]]
</div>
<template is="dom-if" if="[[isSecondaryUser_]]">
<!-- Non primary users. -->
<div class$="settings-box single-column
[[messagesDividerClass_('secondary', managedProperties_,
globalPolicy, managedNetworkAvailable,
isSecondaryUser_, isWifiSyncEnabled_)]]">
<div class="layout horizontal center">
<iron-icon class="policy" icon="cr:group"></iron-icon>
<div class="settings-box-text">
[[i18n('networkPrimaryUserControlled', primaryUserEmail_)]]
</div>
</div>
</template>
</div>
</template>


<template is="dom-if"
if="[[showShared_(managedProperties_, globalPolicy,
managedNetworkAvailable)]]">
<!-- Shared network. -->
<div class$="settings-box settings-box-text
[[messagesDividerClass_('shared', managedProperties_,
globalPolicy, managedNetworkAvailable,
isSecondaryUser_, isWifiSyncEnabled_)]]">
[[sharedString_(managedProperties_)]]
</div>
</template>
<template is="dom-if"
if="[[showSynced_(managedProperties_, globalPolicy,
managedNetworkAvailable, isWifiSyncEnabled_)]]">
<!-- Synced network. -->
<div class$="settings-box settings-box-text
[[messagesDividerClass_('synced', managedProperties_,
globalPolicy, managedNetworkAvailable,
isSecondaryUser_, isWifiSyncEnabled_)]]">
<settings-localized-link
localized-string="[[syncedString_(managedProperties_)]]">
</settings-localized-link>
</div>
</template>
<!-- End of NOTICES section -->
<template is="dom-if"
if="[[showShared_(managedProperties_, globalPolicy,
managedNetworkAvailable)]]">
<!-- Shared network. -->
<div class$="settings-box settings-box-text
[[messagesDividerClass_('shared', managedProperties_,
globalPolicy, managedNetworkAvailable,
isSecondaryUser_, isWifiSyncEnabled_)]]">
[[sharedString_(managedProperties_)]]
</div>
</template>
<template is="dom-if"
if="[[showSynced_(managedProperties_, globalPolicy,
managedNetworkAvailable, isWifiSyncEnabled_)]]">
<!-- Synced network. -->
<div class$="settings-box settings-box-text
[[messagesDividerClass_('synced', managedProperties_,
globalPolicy, managedNetworkAvailable,
isSecondaryUser_, isWifiSyncEnabled_)]]">
<settings-localized-link
localized-string="[[syncedString_(managedProperties_)]]">
</settings-localized-link>
</div>
</template>
<!-- End of NOTICES section -->

<template is="dom-if" if="[[!isSecondaryUser_]]">
<template is="dom-if" if="[[showConfigurableSections_]]" restamp>
Expand Down
5 changes: 2 additions & 3 deletions chrome/test/data/webui/chromeos/internet_detail_dialog_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ suite('internet-detail-dialog', () => {
assertTrue(internetDetailDialog.showCellularSim_(managedProperties));
assertFalse(!!internetDetailDialog.$$('network-siminfo'));

assertFalse(!!internetDetailDialog.$$('cr-button'));
// The 'Forget' and 'ConnectDisconnect' buttons should still be showing.
assertTrue(!!internetDetailDialog.$$('cr-button'));
});

test('Network on active sim, show configurations', async () => {
Expand All @@ -134,8 +135,6 @@ suite('internet-detail-dialog', () => {
const managedProperties = internetDetailDialog.managedProperties_;
assertTrue(internetDetailDialog.showCellularSim_(managedProperties));
assertTrue(!!internetDetailDialog.$$('network-siminfo'));

assertTrue(!!internetDetailDialog.$$('cr-button'));
});

test('Dialog disabled when inhibited', async () => {
Expand Down
7 changes: 3 additions & 4 deletions chrome/test/data/webui/settings/chromeos/internet_detail_page_tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,8 +575,6 @@ suite('InternetDetailPage', function() {
});
await flushAsync();
assertTrue(internetDetailPage.showConfigurableSections_);
// Check that an element from first dom-if exists
assertTrue(!!internetDetailPage.$$('#connectDisconnect'));
// Check that an element from the primary account section exists.
assertTrue(!!internetDetailPage.$$('#allowDataRoaming'));
});
Expand Down Expand Up @@ -610,10 +608,11 @@ suite('InternetDetailPage', function() {
});
await flushAsync();
assertFalse(internetDetailPage.showConfigurableSections_);
// Check that an element from first dom-if exists
assertFalse(!!internetDetailPage.$$('#connectDisconnect'));
// Check that an element from the primary account section exists.
assertFalse(!!internetDetailPage.$$('#allowDataRoaming'));
// The section ConnectDisconnect button belongs to should still be
// showing.
assertTrue(!!internetDetailPage.$$('#connectDisconnect'));
});

test('Hide config section when sim becomes non-active', async () => {
Expand Down

0 comments on commit 7dac310

Please sign in to comment.