luci-base: correctness fixes

validation:

use correct argument position for apply

network:

spec.need_tag -> port.need_tag agrees with old lua compat

widgets:

rv.length is undefined, use firstChild

form:

Follow-up to 315dbfc749
checkDepends recursion fix and implement cache lookup

uci:

improve timeout and Promise handling

ui:

follow-up to 92381c3ca2
renderListing sort: put directories first
getActiveTabId: check isNaN for tab state
getScrollParent: fix evaluation logic
fadeOutNotification: implement immediate timeout
openDropdown: accelerate draw via getBoundingClientRect

form:
ensure FlagValue parse always resolves

fs:
parse all 'expect' keys in RpcReply

luci:

Use the same source of truth in both the check and the dispatch
for flushRequestQueue

string check for dom string additions

Signed-off-by: Paul Donald <newtwen+github@gmail.com>

modules/luci-base/htdocs/luci-static/resources/firewall.js

@@ -319,7 +319,7 @@ Zone = AbstractFirewallItem.extend({
 			this.data = section;
 		}
 		else if (name != null) {
-			var sections = uci.get('firewall', 'zone');
+			var sections = uci.sections('firewall', 'zone');
 
 			for (var i = 0; i < sections.length; i++) {
 				if (sections[i].name != name)

modules/luci-base/htdocs/luci-static/resources/form.js

@@ -7,7 +7,7 @@
 
 const scope = this;
 
-uci.loadPackage('luci').catch();
+uci.loadPackage('luci').catch(() => {});
 
 const callSessionAccess = rpc.declare({
 	object: 'session',
@@ -752,15 +752,18 @@ const CBIMap = CBIAbstractElement.extend(/** @lends LuCI.form.Map.prototype */ {
 	 * @param {Event} ev
 	 * @param {number} n
 	 */
-	checkDepends(ev, n) {
+	checkDepends(ev, n, cache) {
+		if (cache == null)
+			cache = Object.create(null);
+
 		let changed = false;
 
 		for (let i = 0, s = this.children[0]; (s = this.children[i]) != null; i++)
-			if (s.checkDepends(ev, n))
+			if (s.checkDepends(ev, n, cache))
 				changed = true;
 
 		if (changed && (n ?? 0) < 10)
-			this.checkDepends(ev, (n ?? 10) + 1);
+			this.checkDepends(ev, (n ?? 0) + 1, cache);
 
 		ui.tabs.updateTabs(ev, this.root);
 	},
@@ -772,9 +775,12 @@ const CBIMap = CBIAbstractElement.extend(/** @lends LuCI.form.Map.prototype */ {
 	 * @param {string} section_id
 	 * @returns {boolean}
 	 */
-	isDependencySatisfied(depends, config_name, section_id) {
+	isDependencySatisfied(depends, config_name, section_id, cache) {
 		let def = false;
 
+		if (cache == null)
+			cache = Object.create(null);
+
 		if (!Array.isArray(depends) || !depends.length)
 			return true;
 
@@ -792,8 +798,17 @@ const CBIMap = CBIAbstractElement.extend(/** @lends LuCI.form.Map.prototype */ {
 					istat = false;
 				}
 				else {
-					const res = this.lookupOption(dep, section_id, config_name);
-					const val = (res && res[0].isActive(res[1])) ? res[0].formvalue(res[1]) : null;
+					const key = `${config_name}::${section_id}::${dep}`;
+					let val;
+
+					if (key in cache) {
+						val = cache[key];
+					}
+					else {
+						const res = this.lookupOption(dep, section_id, config_name);
+						val = (res && res[0].isActive(res[1])) ? res[0].formvalue(res[1]) : null;
+						cache[key] = val;
+					}
 
 					const equal = contains
 						? isContained(val, depends[i][dep])
@@ -1782,9 +1797,9 @@ const CBIAbstractValue = CBIAbstractElement.extend(/** @lends LuCI.form.Abstract
 	 * @param {string} section_id
 	 * @returns {boolean}
 	 */
-	checkDepends(section_id) {
+	checkDepends(section_id, cache) {
 		const config_name = this.uciconfig ?? this.section.uciconfig ?? this.map.config;
-		const active = this.map.isDependencySatisfied(this.deps, config_name, section_id);
+		const active = this.map.isDependencySatisfied(this.deps, config_name, section_id, cache);
 
 		if (active)
 			this.updateDefaultValue(section_id);
@@ -5158,6 +5173,7 @@ const CBIFlagValue = CBIValue.extend(/** @lends LuCI.form.Flag.prototype */ {
 		else if (!this.retain) {
 			return Promise.resolve(this.remove(section_id));
 		}
+		return Promise.resolve();
 	},
 });
 
@@ -6095,9 +6111,9 @@ const CBISectionValue = CBIValue.extend(/** @lends LuCI.form.SectionValue.protot
 	 * @param {string} section_id
 	 * @returns {null}
 	 */
-	checkDepends(section_id) {
-		this.subsection.checkDepends(section_id);
-		return CBIValue.prototype.checkDepends.apply(this, [ section_id ]);
+	checkDepends(section_id, cache) {
+		this.subsection.checkDepends(section_id, cache);
+		return CBIValue.prototype.checkDepends.apply(this, [ section_id, cache ]);
 	},
 
 	/**

modules/luci-base/htdocs/luci-static/resources/fs.js

@@ -109,8 +109,6 @@ function handleRpcReply(expect, rc) {
 				let e = new Error(_('Unexpected reply data format')); e.name = 'TypeError';
 				throw e;
 			}
-
-			break;
 		}
 	}
 

modules/luci-base/htdocs/luci-static/resources/luci.js

@@ -257,7 +257,7 @@
 					res = res.apply(this, callArgs);
 
 					if (symStack && symStack.length > 1)
-						symStack.shift(protoCtx);
+						symStack.shift();
 					else
 						delete superContext[slotIdx];
 				}
@@ -571,8 +571,9 @@
 		}
 
 		requestQueue.length = 0;
+		const requestBaseURL = Request.expandURL(classes.rpc.getBaseURL());
 
-		Request.request(rpcBaseURL, reqopt).then(reply => {
+		Request.request(requestBaseURL, reqopt).then(reply => {
 			let json = null, req = null;
 
 			try { json = reply.json() }
@@ -1583,7 +1584,7 @@
 			else if (this.elem(html)) {
 				elem = html;
 			}
-			else if (html.charCodeAt(0) === 60) {
+			else if (typeof(html) === 'string' && html.charCodeAt(0) === 60) {
 				elem = this.parse(html);
 			}
 			else {
@@ -2673,6 +2674,9 @@
 		 * has no sub-features.
 		 */
 		hasSystemFeature() {
+			if (!this.isObject(sysFeatures))
+				return null;
+
 			const ft = sysFeatures[arguments[0]];
 
 			if (arguments.length == 2)
@@ -2831,7 +2835,7 @@
 		 * @returns {string}
 		 * Return the joined URL path.
 		 */
-		path(prefix = '', parts) {
+		path(prefix = '', ...parts) {
 			const url = [ prefix ];
 
 			for (let i = 0; i < parts.length; i++){				

modules/luci-base/htdocs/luci-static/resources/network.js

@@ -468,7 +468,7 @@ function initNetworkState(refresh) {
 
 								if (port.device != null) {
 									spec.device = port.device;
-									spec.tagged = spec.need_tag;
+									spec.tagged = port.need_tag;
 									netdevs[port.num] = port.device;
 								}
 

modules/luci-base/htdocs/luci-static/resources/tools/widgets.js

@@ -207,10 +207,10 @@ var CBIZoneSelect = form.ListValue.extend({
 						emptyval.parentNode.removeChild(emptyval);
 				}
 				else {
-					const anyval = node.querySelector('[data-value="*"]') || '';
-					let emptyval = node.querySelector('[data-value=""]') || '';
+					const anyval = node.querySelector('[data-value="*"]');
+					let emptyval = node.querySelector('[data-value=""]');
 
-					if (emptyval == null && anyval) {
+					if (!emptyval && anyval) {
 						emptyval = anyval.cloneNode(true);
 						emptyval.removeAttribute('display');
 						emptyval.removeAttribute('selected');
@@ -559,7 +559,7 @@ var CBINetworkSelect = form.ListValue.extend({
 			if (values.indexOf(name) == -1)
 				continue;
 
-			if (rv.length)
+			if (rv.firstChild)
 				L.dom.append(rv, ' ');
 
 			L.dom.append(rv, this.renderIfaceBadge(network));

modules/luci-base/htdocs/luci-static/resources/uci.js

@@ -988,32 +988,33 @@ return baseclass.extend(/** @lends LuCI.uci.prototype */ {
 	 * @returns {Promise<number>}
 	 * Returns a promise resolving/rejecting with the `ubus` RPC status code.
 	 */
-	apply(timeout) {
+	apply(timeout = 10) {
 		const self = this;
-		const date = new Date();
 
-		if (typeof(timeout) != 'number' || timeout < 1)
+		if (typeof timeout !== 'number' || timeout < 1)
 			timeout = 10;
 
 		return self.callApply(timeout, true).then(rv => {
 			if (rv != 0)
 				return Promise.reject(rv);
 
-			const try_deadline = date.getTime() + 1000 * timeout;
-			const try_confirm = () => {
-				return self.callConfirm().then(rv => {
-					if (rv != 0) {
-						if (date.getTime() < try_deadline)
+			const try_deadline = Date.now() + timeout * 1000;
+
+			return new Promise((resolve, reject) => {
+				const try_confirm = () => {
+					self.callConfirm().then(rv => {
+						if (rv === 0)
+							return resolve(rv);
+
+						if (Date.now() < try_deadline)
 							window.setTimeout(try_confirm, 250);
 						else
-							return Promise.reject(rv);
-					}
+							reject(rv);
+					}).catch(reject);
+				};
 
-					return rv;
-				});
-			};
-
-			window.setTimeout(try_confirm, 1000);
+				window.setTimeout(try_confirm, 1000);
+			});
 		});
 	},
 

modules/luci-base/htdocs/luci-static/resources/ui.js

@@ -1265,24 +1265,18 @@ const UIDropdown = UIElement.extend(/** @lends LuCI.ui.Dropdown.prototype */ {
 	 * @returns {document}
 	 */
 	getScrollParent(element) {
-		let parent = element;
-		let style = getComputedStyle(element);
-		const excludeStaticParent = (style.position === 'absolute');
+		let parent = element.parentElement;
 
-		if (style.position === 'fixed')
-			return document.body;
-
-		while ((parent = parent.parentElement) != null) {
-			style = getComputedStyle(parent);
-
-			if (excludeStaticParent && style.position === 'static')
-				continue;
+		while (parent) {
+			const style = getComputedStyle(parent);
 
 			if (/(auto|scroll)/.test(style.overflow + style.overflowY + style.overflowX))
 				return parent;
+
+			parent = parent.parentElement;
 		}
 
-		return document.body;
+		return document.scrollingElement || document.documentElement;
 	},
 
 
@@ -1349,14 +1343,12 @@ const UIDropdown = UIElement.extend(/** @lends LuCI.ui.Dropdown.prototype */ {
 
 			window.requestAnimationFrame(() => {
 				const containerRect = scrollParent.getBoundingClientRect();
-				const itemHeight = li[Math.max(0, li.length - 2)].getBoundingClientRect().height;
-				let fullHeight = 0;
+				const itemHeight = li.length ? li[Math.max(0, li.length - 2)].getBoundingClientRect().height : 0;
+				const visibleItems = (items == -1 ? li.length : items);
+				const fullHeight = itemHeight * visibleItems;
 				const spaceAbove = rect.top - containerRect.top;
 				const spaceBelow = containerRect.bottom - rect.bottom;
 
-				for (let i = 0; i < (items == -1 ? li.length : items); i++)
-					fullHeight += li[i].getBoundingClientRect().height;
-
 				if (fullHeight <= spaceBelow) {
 					ul.style.top = `${rect.height}px`;
 					ul.style.maxHeight = `${spaceBelow}px`;
@@ -1597,7 +1589,7 @@ const UIDropdown = UIElement.extend(/** @lends LuCI.ui.Dropdown.prototype */ {
 	/**
 	 * @private
 	 * @param {Node} sb
-	 * @param {string[]} values
+	 * @param {Object<string, boolean>} values
 	 */
 	setValues(sb, values) {
 		const ul = sb.querySelector('ul');
@@ -1927,7 +1919,7 @@ const UIDropdown = UIElement.extend(/** @lends LuCI.ui.Dropdown.prototype */ {
 					const li = active.nextElementSibling;
 					this.setFocus(sb, li);
 					if (this.options.create && li == li.parentNode.lastElementChild) {
-						const input = li.querySelector('input:not([type="hidden"]):not([type="checkbox"]');
+						const input = li.querySelector('input:not([type="hidden"]):not([type="checkbox"])');
 						if (input) input.focus();
 					}
 					ev.preventDefault();
@@ -3461,8 +3453,7 @@ const UIFileUpload = UIElement.extend(/** @lends LuCI.ui.FileUpload.prototype */
 		const rows = E('ul');
 
 		list.sort((a, b) => {
-			return L.naturalCompare(a.type == 'directory', b.type == 'directory') ||
-				   L.naturalCompare(a.name, b.name);
+			return (b.type == 'directory') - (a.type == 'directory') || L.naturalCompare(a.name, b.name);
 		});
 
 		for (let i = 0; i < list.length; i++) {
@@ -4452,7 +4443,7 @@ const UI = baseclass.extend(/** @lends LuCI.ui.prototype */ {
 					if (element.parentNode) {
 						element.parentNode.removeChild(element);
 					}
-				});
+				}, 0);
 			}
 		}
 
@@ -4809,7 +4800,7 @@ const UI = baseclass.extend(/** @lends LuCI.ui.prototype */ {
 		getActiveTabId(pane) {
 			const path = this.getPathForPane(pane);
 			const p = +(this.getActiveTabState().paths[path]);
-			return p ?? 0;
+			return isNaN(p) ? 0 : p;
 		},
 
 		/**
@@ -5064,11 +5055,26 @@ const UI = baseclass.extend(/** @lends LuCI.ui.prototype */ {
 
 		return new Promise((resolveFn, rejectFn) => {
 			const img = new Image();
+			let timer = window.setTimeout(() => {
+				timer = null;
+				rejectFn();
+			}, 1000);
 
-			img.onload = resolveFn;
-			img.onerror = rejectFn;
+			img.onload = ev => {
+				if (timer !== null)
+					window.clearTimeout(timer);
+				timer = null;
+				img.onload = img.onerror = null;
+				resolveFn(ev);
+			};
 
-			window.setTimeout(rejectFn, 1000);
+			img.onerror = ev => {
+				if (timer !== null)
+					window.clearTimeout(timer);
+				timer = null;
+				img.onload = img.onerror = null;
+				rejectFn(ev);
+			};
 
 			img.src = target;
 		});

modules/luci-base/htdocs/luci-static/resources/validation.js

@@ -549,7 +549,7 @@ const ValidatorFactory = baseclass.extend(/** @lends LuCI.validation.ValidatorFa
 			const x = parseInt(this.value, 16) | 0;
 			const isll = (((x & 0xffc0) ^ 0xfe80) === 0);
 
-			return this.assert(isll && this.apply('ip6addr', nomask),
+			return this.assert(isll && this.apply('ip6addr', null, nomask),
 				_('valid IPv6 Link Local address'));
 		},
 
@@ -564,7 +564,7 @@ const ValidatorFactory = baseclass.extend(/** @lends LuCI.validation.ValidatorFa
 			const x = parseInt(this.value, 16) | 0;
 			const isula = (((x & 0xfe00) ^ 0xfc00) === 0);
 
-			return this.assert(isula && this.apply('ip6addr', nomask),
+			return this.assert(isula && this.apply('ip6addr', null, nomask),
 				_('valid IPv6 ULA address'));
 		},