Commit ea214ea2 authored by Jordy Greenblatt's avatar Jordy Greenblatt Committed by Commit Bot

CrOS Settings: Don't bind language page onCloseMenu_() outside CrOS/win

http://crbug.com/982375:
The callback onCloseMenu_() is only defined in the language page
JavaScript file in CrOS and Windows however it is bound as a callback
in the HTML file regardless of the OS, causing console warnings [1].

http://crbug.com/982390:
Also, the private functions section of the language_page's JS [1] is
complicated unnecessarily by OS specific blocks for the same
conditions that are separated for no logical reason.

The first patchset in this CL consolidates the OS specific blocks of
complete private methods (i.e. fixes http://crbug.com/982390). The
second patch set prevents the onCloseMenu_() callback from being bound
unless the OS is CrOS or window (i.e. fixes http://crbug.com/982375).

[1] https://imgur.com/a/O5IFCE4

Bug: 982375, 982390
Change-Id: Ic72e7f0e8e1d1680907b4ab8aa99b45f221e4575
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693385Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: default avatarMay Lippert <maybelle@chromium.org>
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675847}
parent d6e6dd4e
...@@ -414,7 +414,10 @@ ...@@ -414,7 +414,10 @@
</if> <!-- _google_chrome or not is_macosx --> </if> <!-- _google_chrome or not is_macosx -->
<cr-lazy-render id="menu"> <cr-lazy-render id="menu">
<template> <template>
<cr-action-menu on-close="onCloseMenu_" <cr-action-menu
<if expr="chromeos or is_win">
on-close="onCloseMenu_"
</if>
class$="[[getMenuClass_(prefs.translate.enabled.value)]]"> class$="[[getMenuClass_(prefs.translate.enabled.value)]]">
<if expr="chromeos or is_win"> <if expr="chromeos or is_win">
<cr-checkbox id="uiLanguageItem" <cr-checkbox id="uiLanguageItem"
......
...@@ -127,8 +127,10 @@ Polymer({ ...@@ -127,8 +127,10 @@ Polymer({
], ],
// </if> // </if>
// <if expr="chromeos or is_win">
/** @private {boolean} */ /** @private {boolean} */
isChangeInProgress_: false, isChangeInProgress_: false,
// </if>
// <if expr="not is_macosx"> // <if expr="not is_macosx">
/** /**
...@@ -313,6 +315,52 @@ Polymer({ ...@@ -313,6 +315,52 @@ Polymer({
onInputMethodOptionsTap_: function(e) { onInputMethodOptionsTap_: function(e) {
this.languageHelper.openInputMethodOptions(e.model.item.id); this.languageHelper.openInputMethodOptions(e.model.item.id);
}, },
// TODO(hsuregan): Remove when SplitSettings is complete.
/**
* Incorporates pageVisibility check along with isProspectiveUILanguage_().
* @param {string} languageCode The language code identifying a language.
* @param {string} prospectiveUILanguage The prospective UI language.
* @return {boolean} True if the given language matches the prospective UI
* pref (which may be different from the actual UI language).
* @private
*/
shouldShowExplanation_: function(languageCode, prospectiveUILanguage) {
return this.pageVisibility &&
this.pageVisibility.uiDisplayedInThisLanguage &&
this.isProspectiveUILanguage_(languageCode, prospectiveUILanguage);
},
/**
* @param {string} id The input method ID.
* @param {string} currentId The ID of the currently enabled input method.
* @return {boolean} True if the IDs match.
* @private
*/
isCurrentInputMethod_: function(id, currentId) {
assert(cr.isChromeOS);
return id == currentId;
},
/**
* @param {string} id The input method ID.
* @param {string} currentId The ID of the currently enabled input method.
* @return {string} The class for the input method item.
* @private
*/
getInputMethodItemClass_: function(id, currentId) {
assert(cr.isChromeOS);
return this.isCurrentInputMethod_(id, currentId) ? 'selected' : '';
},
getInputMethodName_: function(id) {
assert(cr.isChromeOS);
const inputMethod =
this.languages.inputMethods.enabled.find(function(inputMethod) {
return inputMethod.id == id;
});
return inputMethod ? inputMethod.displayName : '';
},
// </if> // </if>
// <if expr="chromeos or is_win"> // <if expr="chromeos or is_win">
...@@ -404,6 +452,42 @@ Polymer({ ...@@ -404,6 +452,42 @@ Polymer({
this.closeMenuSoon_(); this.closeMenuSoon_();
}, },
/**
* Checks whether the prospective UI language (the pref that indicates what
* language to use in Chrome) matches the current language. This pref is used
* only on Chrome OS and Windows; we don't control the UI language elsewhere.
* @param {string} languageCode The language code identifying a language.
* @param {string} prospectiveUILanguage The prospective UI language.
* @return {boolean} True if the given language matches the prospective UI
* pref (which may be different from the actual UI language).
* @private
*/
isProspectiveUILanguage_: function(languageCode, prospectiveUILanguage) {
return languageCode == prospectiveUILanguage;
},
/**
* @param {string} prospectiveUILanguage
* @return {string}
* @private
*/
getProspectiveUILanguageName_: function(prospectiveUILanguage) {
return this.languageHelper.getLanguage(prospectiveUILanguage).displayName;
},
/**
* Handler for the restart button.
* @private
*/
onRestartTap_: function() {
// <if expr="chromeos">
settings.LifetimeBrowserProxyImpl.getInstance().signOutAndRestart();
// </if>
// <if expr="is_win">
settings.LifetimeBrowserProxyImpl.getInstance().restart();
// </if>
},
// </if> // </if>
/** /**
...@@ -493,48 +577,6 @@ Polymer({ ...@@ -493,48 +577,6 @@ Polymer({
this.languageHelper.disableLanguage(this.detailLanguage_.language.code); this.languageHelper.disableLanguage(this.detailLanguage_.language.code);
}, },
// <if expr="chromeos or is_win">
/**
* Checks whether the prospective UI language (the pref that indicates what
* language to use in Chrome) matches the current language. This pref is used
* only on Chrome OS and Windows; we don't control the UI language elsewhere.
* @param {string} languageCode The language code identifying a language.
* @param {string} prospectiveUILanguage The prospective UI language.
* @return {boolean} True if the given language matches the prospective UI
* pref (which may be different from the actual UI language).
* @private
*/
isProspectiveUILanguage_: function(languageCode, prospectiveUILanguage) {
return languageCode == prospectiveUILanguage;
},
// <if expr="chromeos">
// TODO(hsuregan): Remove when SplitSettings is complete.
/**
* Incorporates pageVisibility check along with isProspectiveUILanguage_().
* @param {string} languageCode The language code identifying a language.
* @param {string} prospectiveUILanguage The prospective UI language.
* @return {boolean} True if the given language matches the prospective UI
* pref (which may be different from the actual UI language).
* @private
*/
shouldShowExplanation_: function(languageCode, prospectiveUILanguage) {
return this.pageVisibility &&
this.pageVisibility.uiDisplayedInThisLanguage &&
this.isProspectiveUILanguage_(languageCode, prospectiveUILanguage);
},
// </if>
/**
* @param {string} prospectiveUILanguage
* @return {string}
* @private
*/
getProspectiveUILanguageName_: function(prospectiveUILanguage) {
return this.languageHelper.getLanguage(prospectiveUILanguage).displayName;
},
// </if>
/** /**
* @return {string} * @return {string}
* @private * @private
...@@ -751,39 +793,6 @@ Polymer({ ...@@ -751,39 +793,6 @@ Polymer({
return undefined; return undefined;
}, },
// <if expr="chromeos">
/**
* @param {string} id The input method ID.
* @param {string} currentId The ID of the currently enabled input method.
* @return {boolean} True if the IDs match.
* @private
*/
isCurrentInputMethod_: function(id, currentId) {
assert(cr.isChromeOS);
return id == currentId;
},
/**
* @param {string} id The input method ID.
* @param {string} currentId The ID of the currently enabled input method.
* @return {string} The class for the input method item.
* @private
*/
getInputMethodItemClass_: function(id, currentId) {
assert(cr.isChromeOS);
return this.isCurrentInputMethod_(id, currentId) ? 'selected' : '';
},
getInputMethodName_: function(id) {
assert(cr.isChromeOS);
const inputMethod =
this.languages.inputMethods.enabled.find(function(inputMethod) {
return inputMethod.id == id;
});
return inputMethod ? inputMethod.displayName : '';
},
// </if>
/** /**
* @param {!Event} e * @param {!Event} e
* @private * @private
...@@ -834,21 +843,6 @@ Polymer({ ...@@ -834,21 +843,6 @@ Polymer({
}, settings.kMenuCloseDelay); }, settings.kMenuCloseDelay);
}, },
// <if expr="chromeos or is_win">
/**
* Handler for the restart button.
* @private
*/
onRestartTap_: function() {
// <if expr="chromeos">
settings.LifetimeBrowserProxyImpl.getInstance().signOutAndRestart();
// </if>
// <if expr="is_win">
settings.LifetimeBrowserProxyImpl.getInstance().restart();
// </if>
},
// </if>
/** /**
* Toggles the expand button within the element being listened to. * Toggles the expand button within the element being listened to.
* @param {!Event} e * @param {!Event} e
......
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