WebUI NTP: most-visited, disable save in tile dialog if URL is not valid
Bug: 1069056
Change-Id: Iab2cbb778fbd97c5533388949dd7eb03014ff5fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568542
Auto-Submit: Esmael Elmoslimany <aee@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832872}
diff --git a/chrome/browser/resources/new_tab_page/most_visited.html b/chrome/browser/resources/new_tab_page/most_visited.html
index 1a90986..21ebb56 100644
--- a/chrome/browser/resources/new_tab_page/most_visited.html
+++ b/chrome/browser/resources/new_tab_page/most_visited.html
@@ -235,7 +235,8 @@
value="{{dialogTileTitle_}}" spellcheck="false" autofocus></cr-input>
<cr-input id="dialogInputUrl" label="$i18n{urlField}"
value="{{dialogTileUrl_}}" invalid="[[dialogTileUrlInvalid_]]"
- error-message="$i18n{invalidUrl}" spellcheck="false" type="url">
+ error-message="$i18n{invalidUrl}" spellcheck="false" type="url"
+ on-blur="onDialogTileUrlBlur_">
</cr-input>
</div>
<div slot="button-container">
@@ -243,7 +244,7 @@
$i18n{linkCancel}
</cr-button>
<cr-button class="action-button" on-click="onSave_"
- disabled$="[[!dialogTileUrl_]]">
+ disabled$="[[dialogSaveDisabled_]]">
$i18n{linkDone}
</cr-button>
</div>
diff --git a/chrome/browser/resources/new_tab_page/most_visited.js b/chrome/browser/resources/new_tab_page/most_visited.js
index a2b766e461..0d30802a 100644
--- a/chrome/browser/resources/new_tab_page/most_visited.js
+++ b/chrome/browser/resources/new_tab_page/most_visited.js
@@ -66,6 +66,25 @@
r => x >= r.left && x <= r.right && y >= r.top && y <= r.bottom);
}
+
+/**
+ * Returns null if URL is not valid.
+ * @param {string} urlString
+ * @return {URL}
+ * @private
+ */
+function normalizeUrl(urlString) {
+ try {
+ const url = new URL(
+ urlString.includes('://') ? urlString : `https://${urlString}/`);
+ if (['http:', 'https:'].includes(url.protocol)) {
+ return url;
+ }
+ } catch (e) {
+ }
+ return null;
+}
+
class MostVisitedElement extends PolymerElement {
static get is() {
return 'ntp-most-visited';
@@ -126,6 +145,12 @@
/** @private */
dialogTitle_: String,
+ /** @private */
+ dialogSaveDisabled_: {
+ type: Boolean,
+ computed: 'computeDialogSaveDisabled_(dialogTitle_, dialogTileUrl_)',
+ },
+
/**
* Used to hide hover style and cr-icon-button of tiles while the tiles
* are being reordered.
@@ -341,6 +366,14 @@
}
/**
+ * @return {boolean}
+ * @private
+ */
+ computeDialogSaveDisabled_() {
+ return !this.dialogTileUrl_ || normalizeUrl(this.dialogTileUrl_) === null;
+ }
+
+ /**
* If a pointer is over a tile rect that is different from the one being
* dragged, the dragging tile is moved to the new position. The reordering
* is done in the DOM and the by the |reorderMostVisitedTile()| call. This is
@@ -558,6 +591,14 @@
}
/** @private */
+ onDialogTileUrlBlur_() {
+ if (this.dialogTileUrl_.length > 0 &&
+ normalizeUrl(this.dialogTileUrl_) === null) {
+ this.dialogTileUrlInvalid_ = true;
+ }
+ }
+
+ /** @private */
onDialogTileUrlChange_() {
this.dialogTileUrlInvalid_ = false;
}
@@ -642,36 +683,21 @@
/** @private */
async onSave_() {
- let newUrl;
- try {
- newUrl = new URL(
- this.dialogTileUrl_.includes('://') ?
- this.dialogTileUrl_ :
- `https://${this.dialogTileUrl_}/`);
- if (!['http:', 'https:'].includes(newUrl.protocol)) {
- throw new Error();
- }
- } catch (e) {
- this.dialogTileUrlInvalid_ = true;
- return;
- }
-
- this.dialogTileUrlInvalid_ = false;
-
+ const newUrl = {url: normalizeUrl(this.dialogTileUrl_).href};
this.$.dialog.close();
let newTitle = this.dialogTileTitle_.trim();
if (newTitle.length === 0) {
newTitle = this.dialogTileUrl_;
}
if (this.adding_) {
- const {success} = await this.pageHandler_.addMostVisitedTile(
- {url: newUrl.href}, newTitle);
+ const {success} =
+ await this.pageHandler_.addMostVisitedTile(newUrl, newTitle);
this.toast_(success ? 'linkAddedMsg' : 'linkCantCreate', success);
} else {
const {url, title} = this.tiles_[this.actionMenuTargetIndex_];
- if (url.url !== newUrl.href || title !== newTitle) {
+ if (url.url !== newUrl.url || title !== newTitle) {
const {success} = await this.pageHandler_.updateMostVisitedTile(
- url, {url: newUrl.href}, newTitle);
+ url, newUrl, newTitle);
this.toast_(success ? 'linkEditedMsg' : 'linkCantEdit', success);
}
this.actionMenuTargetIndex_ = -1;
diff --git a/chrome/test/data/webui/new_tab_page/most_visited_test.js b/chrome/test/data/webui/new_tab_page/most_visited_test.js
index f1886e7..b489bfb 100644
--- a/chrome/test/data/webui/new_tab_page/most_visited_test.js
+++ b/chrome/test/data/webui/new_tab_page/most_visited_test.js
@@ -445,10 +445,12 @@
});
test('http is a valid scheme', async () => {
+ assertTrue(saveButton.disabled);
inputUrl.value = 'http://url';
const addCalled = testProxy.handler.whenCalled('addMostVisitedTile');
saveButton.click();
await addCalled;
+ assertFalse(saveButton.disabled);
});
test('https is a valid scheme', async () => {
@@ -459,16 +461,18 @@
});
test('chrome is not a valid scheme', () => {
+ assertTrue(saveButton.disabled);
inputUrl.value = 'chrome://url';
assertFalse(inputUrl.invalid);
- saveButton.click();
+ mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur'));
assertTrue(inputUrl.invalid);
+ assertTrue(saveButton.disabled);
});
test('invalid cleared when text entered', () => {
inputUrl.value = '%';
assertFalse(inputUrl.invalid);
- saveButton.click();
+ mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur'));
assertTrue(inputUrl.invalid);
inputUrl.value = '';
assertFalse(inputUrl.invalid);
@@ -533,7 +537,7 @@
inputUrl.value = 'updated-url';
assertFalse(mostVisited.$.toast.open);
saveButton.click();
- await flushTasks();
+ await testProxy.handler.whenCalled('updateMostVisitedTile');
assertTrue(mostVisited.$.toast.open);
});