Commit 65cf7162 authored by Esmael El-Moslimany's avatar Esmael El-Moslimany Committed by Chromium LUCI CQ

WebUI NTP: most-visited add/edit handles shortcut already exists

Bug: 1123905
Change-Id: Icd32d8adb20cb5e8ce16093b50c429ba25758ee9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2573338
Commit-Queue: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833603}
parent 64187e1b
...@@ -5437,6 +5437,9 @@ Keep your key file in a safe place. You will need it to create new versions of y ...@@ -5437,6 +5437,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_NTP_CONFIRM_MSG_SHORTCUT_ADDED" desc="The title of the shortcut added confirmation message when adding a custom link. (On the New Tab Page)"> <message name="IDS_NTP_CONFIRM_MSG_SHORTCUT_ADDED" desc="The title of the shortcut added confirmation message when adding a custom link. (On the New Tab Page)">
Shortcut added Shortcut added
</message> </message>
<message name="IDS_NTP_CUSTOM_LINKS_ALREADY_EXISTS" desc="When adding/editing a custom link, this error is shown when the URL already exists in another custom link. (On the New Tab Page)">
Shortcut already exists
</message>
<message name="IDS_NTP_CONFIRM_MSG_RESTORE_DEFAULTS" desc="The text label of the restore default shortcuts button when editing a custom link. (On the New Tab Page)"> <message name="IDS_NTP_CONFIRM_MSG_RESTORE_DEFAULTS" desc="The text label of the restore default shortcuts button when editing a custom link. (On the New Tab Page)">
Restore default shortcuts Restore default shortcuts
</message> </message>
......
4fefa22bd37edcf1d7019e4e7c8743da87a55571
\ No newline at end of file
...@@ -235,7 +235,7 @@ ...@@ -235,7 +235,7 @@
value="{{dialogTileTitle_}}" spellcheck="false" autofocus></cr-input> value="{{dialogTileTitle_}}" spellcheck="false" autofocus></cr-input>
<cr-input id="dialogInputUrl" label="$i18n{urlField}" <cr-input id="dialogInputUrl" label="$i18n{urlField}"
value="{{dialogTileUrl_}}" invalid="[[dialogTileUrlInvalid_]]" value="{{dialogTileUrl_}}" invalid="[[dialogTileUrlInvalid_]]"
error-message="$i18n{invalidUrl}" spellcheck="false" type="url" error-message="[[dialogTileUrlError_]]" spellcheck="false" type="url"
on-blur="onDialogTileUrlBlur_"> on-blur="onDialogTileUrlBlur_">
</cr-input> </cr-input>
</div> </div>
......
...@@ -148,7 +148,21 @@ class MostVisitedElement extends PolymerElement { ...@@ -148,7 +148,21 @@ class MostVisitedElement extends PolymerElement {
/** @private */ /** @private */
dialogSaveDisabled_: { dialogSaveDisabled_: {
type: Boolean, type: Boolean,
computed: 'computeDialogSaveDisabled_(dialogTitle_, dialogTileUrl_)', computed: `computeDialogSaveDisabled_(dialogTitle_, dialogTileUrl_,
dialogShortcutAlreadyExists_)`,
},
/** @private */
dialogShortcutAlreadyExists_: {
type: Boolean,
computed: 'computeDialogShortcutAlreadyExists_(tiles_, dialogTileUrl_)',
},
/** @private */
dialogTileUrlError_: {
type: String,
computed: `computeDialogTileUrlError_(dialogTileUrl_,
dialogShortcutAlreadyExists_)`,
}, },
/** /**
...@@ -370,7 +384,36 @@ class MostVisitedElement extends PolymerElement { ...@@ -370,7 +384,36 @@ class MostVisitedElement extends PolymerElement {
* @private * @private
*/ */
computeDialogSaveDisabled_() { computeDialogSaveDisabled_() {
return !this.dialogTileUrl_ || normalizeUrl(this.dialogTileUrl_) === null; return !this.dialogTileUrl_ || normalizeUrl(this.dialogTileUrl_) === null ||
this.dialogShortcutAlreadyExists_;
}
/**
* @return {boolean}
* @private
*/
computeDialogShortcutAlreadyExists_() {
const dialogTileHref = (normalizeUrl(this.dialogTileUrl_) || {}).href;
if (!dialogTileHref) {
return false;
}
return (this.tiles_ || []).some(({url: {url}}, index) => {
if (index === this.actionMenuTargetIndex_) {
return false;
}
const otherUrl = normalizeUrl(url);
return otherUrl && otherUrl.href === dialogTileHref;
});
}
/**
* @return {string}
* @private
*/
computeDialogTileUrlError_() {
return loadTimeData.getString(
this.dialogShortcutAlreadyExists_ ? 'shortcutAlreadyExists' :
'invalidUrl');
} }
/** /**
...@@ -593,7 +636,8 @@ class MostVisitedElement extends PolymerElement { ...@@ -593,7 +636,8 @@ class MostVisitedElement extends PolymerElement {
/** @private */ /** @private */
onDialogTileUrlBlur_() { onDialogTileUrlBlur_() {
if (this.dialogTileUrl_.length > 0 && if (this.dialogTileUrl_.length > 0 &&
normalizeUrl(this.dialogTileUrl_) === null) { (normalizeUrl(this.dialogTileUrl_) === null ||
this.dialogShortcutAlreadyExists_)) {
this.dialogTileUrlInvalid_ = true; this.dialogTileUrlInvalid_ = true;
} }
} }
......
...@@ -129,6 +129,7 @@ content::WebUIDataSource* CreateNewTabPageUiHtmlSource(Profile* profile) { ...@@ -129,6 +129,7 @@ content::WebUIDataSource* CreateNewTabPageUiHtmlSource(Profile* profile) {
{"nameField", IDS_NTP_CUSTOM_LINKS_NAME}, {"nameField", IDS_NTP_CUSTOM_LINKS_NAME},
{"restoreDefaultLinks", IDS_NTP_CONFIRM_MSG_RESTORE_DEFAULTS}, {"restoreDefaultLinks", IDS_NTP_CONFIRM_MSG_RESTORE_DEFAULTS},
{"restoreThumbnailsShort", IDS_NEW_TAB_RESTORE_THUMBNAILS_SHORT_LINK}, {"restoreThumbnailsShort", IDS_NEW_TAB_RESTORE_THUMBNAILS_SHORT_LINK},
{"shortcutAlreadyExists", IDS_NTP_CUSTOM_LINKS_ALREADY_EXISTS},
{"urlField", IDS_NTP_CUSTOM_LINKS_URL}, {"urlField", IDS_NTP_CUSTOM_LINKS_URL},
// Customize button and dialog. // Customize button and dialog.
......
...@@ -101,6 +101,10 @@ suite('NewTabPageMostVisitedTest', () => { ...@@ -101,6 +101,10 @@ suite('NewTabPageMostVisitedTest', () => {
updateScreenWidth(true, true); updateScreenWidth(true, true);
} }
function leaveUrlInput() {
mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur'));
}
suiteSetup(() => { suiteSetup(() => {
loadTimeData.overrideValues({ loadTimeData.overrideValues({
linkRemovedMsg: '', linkRemovedMsg: '',
...@@ -464,7 +468,7 @@ suite('NewTabPageMostVisitedTest', () => { ...@@ -464,7 +468,7 @@ suite('NewTabPageMostVisitedTest', () => {
assertTrue(saveButton.disabled); assertTrue(saveButton.disabled);
inputUrl.value = 'chrome://url'; inputUrl.value = 'chrome://url';
assertFalse(inputUrl.invalid); assertFalse(inputUrl.invalid);
mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur')); leaveUrlInput();
assertTrue(inputUrl.invalid); assertTrue(inputUrl.invalid);
assertTrue(saveButton.disabled); assertTrue(saveButton.disabled);
}); });
...@@ -472,11 +476,30 @@ suite('NewTabPageMostVisitedTest', () => { ...@@ -472,11 +476,30 @@ suite('NewTabPageMostVisitedTest', () => {
test('invalid cleared when text entered', () => { test('invalid cleared when text entered', () => {
inputUrl.value = '%'; inputUrl.value = '%';
assertFalse(inputUrl.invalid); assertFalse(inputUrl.invalid);
mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur')); leaveUrlInput();
assertTrue(inputUrl.invalid); assertTrue(inputUrl.invalid);
assertEquals('Type a valid URL', inputUrl.errorMessage);
inputUrl.value = ''; inputUrl.value = '';
assertFalse(inputUrl.invalid); assertFalse(inputUrl.invalid);
}); });
test('shortcut already exists', async () => {
await addTiles(2);
inputUrl.value = 'b';
assertFalse(inputUrl.invalid);
leaveUrlInput();
assertTrue(inputUrl.invalid);
assertEquals('Shortcut already exists', inputUrl.errorMessage);
inputUrl.value = 'c';
assertFalse(inputUrl.invalid);
leaveUrlInput();
assertFalse(inputUrl.invalid);
inputUrl.value = '%';
assertFalse(inputUrl.invalid);
leaveUrlInput();
assertTrue(inputUrl.invalid);
assertEquals('Type a valid URL', inputUrl.errorMessage);
});
}); });
test('open edit dialog', async () => { test('open edit dialog', async () => {
...@@ -574,6 +597,20 @@ suite('NewTabPageMostVisitedTest', () => { ...@@ -574,6 +597,20 @@ suite('NewTabPageMostVisitedTest', () => {
const [url, newUrl, newTitle] = await updateCalled; const [url, newUrl, newTitle] = await updateCalled;
assertEquals('https://updated-url/', newUrl.url); assertEquals('https://updated-url/', newUrl.url);
}); });
test('shortcut already exists', async () => {
inputUrl.value = 'a';
assertFalse(inputUrl.invalid);
leaveUrlInput();
assertTrue(inputUrl.invalid);
assertEquals('Shortcut already exists', inputUrl.errorMessage);
// The shortcut being editted has a URL of https://b/. Entering the same
// URL is not an error.
inputUrl.value = 'b';
assertFalse(inputUrl.invalid);
leaveUrlInput();
assertFalse(inputUrl.invalid);
});
}); });
test('remove with action menu', async () => { test('remove with action menu', async () => {
......
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