Editor: fixes for 0e391f9, refactor, more fixes

* recognize changes in applies-to inputs
* recognize changes only in specified controls
* forget the dirty applies-to ids from a deleted section after the style was saved
* toMozillaFormat: use the actual contents of CodeMirror instances
* toMozillaFormat: allow empty code section with non-empty applies-to
* refactor: simplify, de-kludge, de-duplicate, de-obfuscate, use more descriptive names instead of "items", "a", "b" where appropriate

edit.html

@@ -193,8 +193,8 @@
 		<div id="header">
 			<h1 id="heading"></h1>
 			<section id="basic-info">
-				<div><label for="name" id="name-label"></label><input id="name"></div>
+				<div><label for="name" id="name-label"></label><input id="name" class="style-contributor"></div>
-				<div><label for="enabled" id="enabled-label"></label><input type="checkbox" id="enabled"></div>
+				<div><label for="enabled" id="enabled-label"></label><input type="checkbox" id="enabled" class="style-contributor"></div>
 			</section>
 			<button id="to-mozilla"></button><img id="to-mozilla-help" src="help.png"><br><br>
 			<button id="save-button" title="Ctrl-S"></button>

edit.js

@@ -1,85 +1,88 @@
 "use strict";
 
 var styleId = null;
-var dirty = false;
+var dirty = {};       // only the actually dirty items here
-var lockScroll; // ensure the section doesn't jump when clicking selected text
+var editors = [];     // array of all CodeMirror instances
-var isSeparateWindow;
+var lockScroll;       // temporary focus-jump-on-click fix, TODO: revert c084ea3 once CM is updated
+var isSeparateWindow; // used currrently to determine if the window size/pos should be remembered
 
+// direct & reverse mapping of @-moz-document keywords and internal property names
+var propertyToCss = {urls: "url", urlPrefixes: "url-prefix", domains: "domain", regexps: "regexp"};
+var CssToProperty = {"url": "urls", "url-prefix": "urlPrefixes", "domain": "domains", "regexp": "regexps"};
+
+// templates
 var appliesToTemplate = document.createElement("li");
-appliesToTemplate.innerHTML = '<select name="applies-type" class="applies-type"><option value="url">' + t("appliesUrlOption") + '</option><option value="url-prefix">' + t("appliesUrlPrefixOption") + '</option><option value="domain">' + t("appliesDomainOption") + '</option><option value="regexp">' + t("appliesRegexpOption") + '</option></select><input name="applies-value" class="applies-value"><button class="remove-applies-to">' + t("appliesRemove") + '</button><button class="add-applies-to">' + t("appliesAdd") + '</button>';
+appliesToTemplate.innerHTML = '<select name="applies-type" class="applies-type style-contributor"><option value="url">' + t("appliesUrlOption") + '</option><option value="url-prefix">' + t("appliesUrlPrefixOption") + '</option><option value="domain">' + t("appliesDomainOption") + '</option><option value="regexp">' + t("appliesRegexpOption") + '</option></select><input name="applies-value" class="applies-value style-contributor"><button class="remove-applies-to">' + t("appliesRemove") + '</button><button class="add-applies-to">' + t("appliesAdd") + '</button>';
 
 var appliesToEverythingTemplate = document.createElement("li");
 appliesToEverythingTemplate.className = "applies-to-everything";
-appliesToEverythingTemplate.innerHTML = t("appliesToEverything") + ' <button class="add-applies-to">' + t("appliesSpecify") + '</button>'
+appliesToEverythingTemplate.innerHTML = t("appliesToEverything") + ' <button class="add-applies-to">' + t("appliesSpecify") + '</button>';
 
 var sectionTemplate = document.createElement("div");
 sectionTemplate.innerHTML = '<label>' + t('sectionCode') + '</label><textarea class="code"></textarea><br><div class="applies-to"><label>' + t("appliesLabel") + ' <img class="applies-to-help" src="help.png" alt="' + t('helpAlt') + '"></label><ul class="applies-to-list"></ul></div><button class="remove-section">' + t('sectionRemove') + '</button><button class="add-section">' + t('sectionAdd') + '</button>';
 
-document.addEventListener("change", function(event) {
+
-	var node = event.target;
+// make querySelectorAll enumeration code readable
-	if (node.type && !node.form) { // INPUTs that aren't in a FORM are stylesheet
+["forEach", "some", "indexOf"].forEach(function(method) {
-		switch (node.type) {
+	NodeList.prototype[method]= Array.prototype[method];
-			case "checkbox":
-				setCleanItem(node, node.checked === node.defaultChecked);
-				break;
-			case "text":
-			case "select-one":
-			case "select-multiple":
-				setCleanItem(node, node.value === node.defaultValue);
-				break;
-		}
-	}
 });
 
+function onChange(event) {
+	var node = event.target;
+	if ("savedValue" in node) {
+		var currentValue = "checkbox" === node.type ? node.checked : node.value;
+		setCleanItem(node, node.savedValue === currentValue);
+	} else {
+		// the manually added section's applies-to is dirty only when the value is non-empty
+		setCleanItem(node, node.localName != "input" || !node.value.trim());
+		delete node.savedValue; // only valid when actually saved
+	}
+	updateTitle();
+}
+
 // Set .dirty on stylesheet contributors that have changed
-var items = {};
+function setDirtyClass(node, isDirty) {
-function isCleanItem(node) {
+	node.classList.toggle("dirty", isDirty);
-	return items[node.id];
 }
-function setCleanItem(node, clean) {
-	var id = node.id;
-	if (!id) id = node.id = Date.now().toString(32).substr(-6);
-	items[id] = clean;
 
-	if (clean) node.classList.remove("dirty");
+function setCleanItem(node, isClean) {
-	else node.classList.add("dirty");
+	if (!node.id) {
-
+		node.id = Date.now().toString(32).substr(-6);
-	initTitle();
 	}
+
+	if (isClean) {
+		delete dirty[node.id];
+		node.savedValue = "checkbox" === node.type ? node.checked : node.value;
+	} else {
+		dirty[node.id] = true;
+	}
+
+	setDirtyClass(node, !isClean);
+}
+
 function isCleanGlobal() {
-	var clean = Object.keys(items)
+	var clean = Object.keys(dirty).length == 0;
-				      .every(function(item) { return items[item] });
+	setDirtyClass(document.body, !clean);
-
-	if (clean) document.body.classList.remove("dirty");
-	else document.body.classList.add("dirty");
-
 	return clean;
 }
-function setCleanGlobal(form) {
+
-	if (!form) form = null;
+function setCleanGlobal() {
-	Array.prototype.forEach.call(document.querySelectorAll("input, select"), function(node) {
+	document.querySelectorAll("#header, #sections > div").forEach(setCleanSection);
-		if (node.form === form) {
+	dirty = {}; // forget the dirty applies-to ids from a deleted section after the style was saved
-			if ("checkbox" === node.type) {
-				node.defaultChecked = node.checked;
-			} else {
-				node.defaultValue = node.value;
 }
 
-			node.classList.remove("dirty");
+function setCleanSection(section) {
-			delete items[node.id];
+	section.querySelectorAll(".style-contributor").forEach(function(node) { setCleanItem(node, true) });
-		}
-	});
 
-	editors.forEach(function(cm) {
+	// #header section has no codemirror
-		cm.lastChange = cm.changeGeneration();
+	var wrapper = section.querySelector(".CodeMirror");
-		cm.getTextArea().parentNode.defaultValue = cm.lastChange;
+	if (wrapper) {
+		var cm = wrapper.CodeMirror;
+		section.savedValue = cm.changeGeneration();
 		indicateCodeChange(cm);
-	});
+	}
-
-	initTitle();
 }
 
-var editors = []; // array of all CodeMirror instances
 function initCodeMirror() {
 	var CM = CodeMirror;
 	// default option values
@@ -106,8 +109,8 @@ function initCodeMirror() {
 	// additional commands
 	var cc = CM.commands;
 	cc.jumpToLine = jumpToLine;
-	cc.nextBuffer = nextBuffer;
+	cc.nextBuffer = function(cm) { nextPrevBuffer(cm, 1) };
-	cc.prevBuffer = prevBuffer;
+	cc.prevBuffer = function(cm) { nextPrevBuffer(cm, -1) };
 
 	var cssHintHandler = CM.hint.css;
 	CM.hint.css = function(cm) {
@@ -163,7 +166,7 @@ function acmeEventListener(event) {
 }
 
 // replace given textarea with the CodeMirror editor
-function setupCodeMirror(textarea) {
+function setupCodeMirror(textarea, index) {
 	var cm = CodeMirror.fromTextArea(textarea);
 	cm.addKeyMap({
 		"Ctrl-G": "jumpToLine",
@@ -184,7 +187,14 @@ function setupCodeMirror(textarea) {
 		}, 0);
 	});
 
-	editors.push(cm);
+	editors.splice(index || editors.length, 0, cm);
+	return cm;
+}
+
+function indicateCodeChange(cm) {
+	var section = cm.getTextArea().parentNode;
+	setCleanItem(section, cm.isClean(section.savedValue));
+	updateTitle();
 }
 
 // ensure the section doesn't jump when clicking selected text
@@ -220,10 +230,6 @@ window.onbeforeunload = function() {
 	return !isCleanGlobal() ? t('styleChangesNotSaved') : null;
 }
 
-function indicateCodeChange(cm) {
-	setCleanItem(cm.getTextArea().parentNode, cm.isClean(cm.lastChange));
-}
-
 function addAppliesTo(list, name, value) {
 	var showingEverything = list.querySelector(".applies-to-everything") != null;
 	// blow away "Everything" if it's there
@@ -255,83 +261,72 @@ function addSection(event, section) {
 	div.querySelector(".remove-section").addEventListener("click", removeSection, false);
 	div.querySelector(".add-section").addEventListener("click", addSection, false);
 
+	var codeElement = div.querySelector(".code");
 	var appliesTo = div.querySelector(".applies-to-list");
+	var appliesToAdded = false;
 
 	if (section) {
-		var codeElement = div.querySelector(".code");
 		codeElement.value = section.code;
-		// codeElement.addEventListener("change", makeDirty, false);
+		for (var i in propertyToCss) {
-		// // Why is this here? Is it possible for CM to not load?
+			if (section[i]) {
-		if (section.urls) {
+				section[i].forEach(function(url) {
-			section.urls.forEach(function(url) {
+					addAppliesTo(appliesTo, propertyToCss[i], url);
-				addAppliesTo(appliesTo, "url", url);
+					appliesToAdded = true;
 				});
 			}
-		if (section.urlPrefixes) {
-			section.urlPrefixes.forEach(function(url) {
-				addAppliesTo(appliesTo, "url-prefix", url);
-			});
 		}
-		if (section.domains) {
-			section.domains.forEach(function(d) {
-				addAppliesTo(appliesTo, "domain", d);
-			});
 	}
-		if (section.regexps) {
+	if (!appliesToAdded) {
-			section.regexps.forEach(function(d) {
-				addAppliesTo(appliesTo, "regexp", d);
-			});
-		}
-		if (!section.urls && !section.urlPrefixes && !section.domains && !section.regexps) {
-			addAppliesTo(appliesTo);
-		}
-	} else {
 		addAppliesTo(appliesTo);
 	}
 
+	appliesTo.addEventListener("change", onChange);
+	appliesTo.addEventListener("input", onChange);
+
 	var sections = document.getElementById("sections");
-	var section = event ? event.target.parentNode : null;
+	if (event) {
-	if (event && section.nextElementSibling) {
+		var clickedSection = event.target.parentNode;
-		sections.insertBefore(div, section.nextElementSibling);
+		sections.insertBefore(div, clickedSection.nextElementSibling);
+		var newIndex = document.querySelectorAll("#sections > div").indexOf(clickedSection) + 1;
+		setupCodeMirror(codeElement, newIndex).focus();
 	} else {
 		sections.appendChild(div);
+		setupCodeMirror(codeElement);
 	}
-	setupCodeMirror(div.querySelector('.code'));
+
-	if (section) {
+	setCleanSection(div);
-		var index = Array.prototype.indexOf.call(sections.children, section);
-		var cm = editors.pop();
-		editors.splice(index, 0, cm);
-		cm.focus();
-	}
 }
 
 function removeAppliesTo(event) {
 	var appliesTo = event.target.parentNode,
 	    appliesToList = appliesTo.parentNode;
-	appliesToList.removeChild(appliesTo);
+	removeAreaAndSetDirty(appliesTo);
 	if (!appliesToList.hasChildNodes()) {
-		var e = appliesToEverythingTemplate.cloneNode(true);
+		addAppliesTo(appliesToList);
-		e.querySelector(".add-applies-to").addEventListener("click", function() {addAppliesTo(this.parentNode.parentNode)}, false);
-		appliesToList.appendChild(e);
 	}
-	Array.prototype.forEach.call(appliesTo.querySelectorAll("input, select"), function(node) {
-		setCleanItem(node, !node.defaultValue);
-	});
 }
 
 function removeSection(event) {
 	var section = event.target.parentNode;
-    var wrapper = section.querySelector(".CodeMirror-wrap");
+	var cm = section.querySelector(".CodeMirror").CodeMirror;
-	var idx = editors.indexOf(wrapper && wrapper.CodeMirror);
+	removeAreaAndSetDirty(section);
-    if (idx >= 0) {
+	editors.splice(editors.indexOf(cm), 1);
-        editors.splice(idx, 1);
+}
-		setCleanItem(wrapper.parentNode, true);
+
+function removeAreaAndSetDirty(area) {
+	area.querySelectorAll('.style-contributor').some(function(node) {
+		if (node.savedValue) {
+			// it's a saved section, so make it dirty and stop the enumeration
+			setCleanItem(area, false);
+			return true;
+		} else {
+			// it's an empty section, so undirty the applies-to items,
+			// otherwise orphaned ids would keep the style dirty
+			setCleanItem(node, true);
 		}
-	section.parentNode.removeChild(section);
-	Array.prototype.forEach.call(section.querySelectorAll("input, select"), function(node) {
-		setCleanItem(node, !node.defaultValue);
 	});
-	setCleanItem(section, !section.defaultValue);
+	updateTitle();
+	area.parentNode.removeChild(area);
 }
 
 function makeSectionVisible(cm) {
@@ -440,13 +435,8 @@ function jumpToLine(cm) {
 	}, {value: cur.line+1});
 }
 
-function nextBuffer(cm) {
+function nextPrevBuffer(cm, direction) {
-	var cm = editors[(editors.indexOf(cm) + 1) % editors.length];
+	cm = editors[(editors.indexOf(cm) + direction + editors.length) % editors.length];
-	makeSectionVisible(cm);
-	cm.focus();
-}
-function prevBuffer(cm) {
-	var cm = editors[(editors.indexOf(cm) - 1 + editors.length) % editors.length];
 	makeSectionVisible(cm);
 	cm.focus();
 }
@@ -459,20 +449,16 @@ function init() {
 	if (!params.id) { // match should be 2 - one for the whole thing, one for the parentheses
 		// This is an add
 		var section = {code: ""}
-		if (params.domain) {
+		for (var i in CssToProperty) {
-			section.domains = [params.domain];
+			if (params[i]) {
-		} else if (params.url) {
+				section[CssToProperty[i]] = [params[i]];
-			section.urls = [params.url];
+			}
-		} else if (params["url-prefix"]) {
-			section.urlPrefixes = [params["url-prefix"]];
 		}
 		addSection(null, section);
 		// default to enabled
 		document.getElementById("enabled").checked = true
 		tE("heading", "addStyleTitle");
-		setupGlobalSearch();
+		initHooks();
-		setCleanGlobal(null);
-		initTitle();
 		return;
 	}
 	// This is an edit
@@ -488,7 +474,7 @@ function initWithStyle(style) {
 	document.getElementById("enabled").checked = style.enabled == "true";
 	document.getElementById("heading").innerHTML = t("editStyleHeading");
 	// if this was done in response to an update, we need to clear existing sections
-	Array.prototype.forEach.call(document.querySelectorAll("#sections > div"), function(div) {
+	document.querySelectorAll("#sections > div").forEach(function(div) {
 		div.parentNode.removeChild(div);
 	});
 	style.sections.forEach(function(section) {
@@ -496,18 +482,27 @@ function initWithStyle(style) {
 			addSection(null, section)
 		}, 0);
 	});
-	setupGlobalSearch();
+	initHooks();
-	setCleanGlobal(null);
-	initTitle();
 }
 
-function initTitle() {
+function initHooks() {
+	document.querySelectorAll("#header .style-contributor").forEach(function(node) {
+		node.addEventListener("change", onChange);
+		node.addEventListener("input", onChange);
+	});
+
+	setupGlobalSearch();
+	setCleanGlobal();
+	updateTitle();
+}
+
+function updateTitle() {
 	const DIRTY_TITLE = "* $";
 
-	var name = document.getElementById("name").defaultValue;
+	var name = document.getElementById("name").savedValue;
-	var dirty = !isCleanGlobal();
+	var clean = isCleanGlobal();
 	var title = styleId === null ? t("addStyleTitle") : t('editStyleTitle', [name]);
-	document.title = !dirty ? title : DIRTY_TITLE.replace("$", title);
+	document.title = clean ? title : DIRTY_TITLE.replace("$", title);
 }
 
 function validate() {
@@ -516,18 +511,18 @@ function validate() {
 		return t("styleMissingName");
 	}
 	// validate the regexps
-	if (Array.prototype.some.call(document.querySelectorAll(".applies-to-list"), function(list) {
+	if (document.querySelectorAll(".applies-to-list").some(function(list) {
-		return Array.prototype.some.call(list.childNodes, function(li) {
+		return list.childNodes.some(function(li) {
 			if (li.className == appliesToEverythingTemplate.className) {
 				return false;
 			}
 			var valueElement = li.querySelector("[name=applies-value]");
-			var a = li.querySelector("[name=applies-type]").value;
+			var type = li.querySelector("[name=applies-type]").value;
-			var b = valueElement.value;
+			var value = valueElement.value;
-			if (a && b) {
+			if (type && value) {
-				if (a == "regexp") {
+				if (type == "regexp") {
 					try {
-						new RegExp(b);
+						new RegExp(value);
 					} catch (ex) {
 						valueElement.focus();
 						return true;
@@ -560,19 +555,19 @@ function save() {
 		id: styleId,
 		name: name,
 		enabled: enabled,
-		sections: getSections(),
+		sections: getSections()
 	};
 	chrome.extension.sendMessage(request, saveComplete);
 }
 
 function getSections() {
 	var sections = [];
-	Array.prototype.forEach.call(document.querySelectorAll("#sections > div"), function(div) {
+	document.querySelectorAll("#sections > div").forEach(function(div) {
-		var code = div.querySelector(".code").value;
+		var meta = getMeta(div);
-		if (/^\s*$/.test(code)) {
+		var code = div.querySelector(".CodeMirror").CodeMirror.getValue();
+		if (/^\s*$/.test(code) && Object.keys(meta).length == 0) {
 			return;
 		}
-		var meta = getMeta(div);
 		meta.code = code;
 		sections.push(meta);
 	});
@@ -580,28 +575,16 @@ function getSections() {
 }
 
 function getMeta(e) {
-	var meta = {urls: [], urlPrefixes: [], domains: [], regexps: []};
+	var meta = {};
-	Array.prototype.forEach.call(e.querySelector(".applies-to-list").childNodes, function(li) {
+	e.querySelector(".applies-to-list").childNodes.forEach(function(li) {
 		if (li.className == appliesToEverythingTemplate.className) {
 			return;
 		}
-		var a = li.querySelector("[name=applies-type]").value;
+		var type = li.querySelector("[name=applies-type]").value;
-		var b = li.querySelector("[name=applies-value]").value;
+		var value = li.querySelector("[name=applies-value]").value;
-		if (a && b) {
+		if (type && value) {
-			switch (a) {
+			var property = CssToProperty[type];
-				case "url":
+			meta[property] ? meta[property].push(value) : meta[property] = [value];
-					meta.urls.push(b);
-					break;
-				case "url-prefix":
-					meta.urlPrefixes.push(b);
-					break;
-				case "domain":
-					meta.domains.push(b);
-					break;
-				case "regexp":
-					meta.regexps.push(b);
-					break;
-			}
 		}
 	});
 	return meta;
@@ -609,34 +592,32 @@ function getMeta(e) {
 
 function saveComplete(style) {
 	styleId = style.id;
-	setCleanGlobal(null);
+	setCleanGlobal();
 
 	// Go from new style URL to edit style URL
 	if (location.href.indexOf("id=") == -1) {
 		// give the code above a moment before we kill the page
 		setTimeout(function() {location.href = "edit.html?id=" + style.id;}, 200);
 	} else {
-		initTitle();
+		updateTitle();
 	}
 }
 
 function showMozillaFormat() {
-	var w = window.open("data:text/plain;charset=UTF-8," + encodeURIComponent(toMozillaFormat()));
+	window.open("data:text/plain;charset=UTF-8," + encodeURIComponent(toMozillaFormat()));
 }
 
 function toMozillaFormat() {
 	return getSections().map(function(section) {
-		if (section.urls.length == 0 && section.urlPrefixes.length == 0 && section.domains.length == 0 && section.regexps.length == 0) {
-			return section.code;
-		}
-		var propertyToCss = {"urls": "url", "urlPrefixes": "url-prefix", "domains": "domain", "regexps": "regexp"};
 		var cssMds = [];
 		for (var i in propertyToCss) {
+			if (section[i]) {
 				cssMds = cssMds.concat(section[i].map(function (v){
 					return propertyToCss[i] + "(\"" + v.replace(/\\/g, "\\\\") + "\")";
 				}));
 			}
-		return "@-moz-document " + cssMds.join(", ") + " {\n" + section.code + "\n}";
+		}
+		return cssMds.length ? "@-moz-document " + cssMds.join(", ") + " {\n" + section.code + "\n}" : section.code;
 	}).join("\n\n");
 }
 
@@ -675,7 +656,6 @@ chrome.extension.onMessage.addListener(function(request, sender, sendResponse) {
 		case "styleUpdated":
 			if (styleId == request.id) {
 				initWithStyle(request.style);
-				dirty = false;
 			}
 			break;
 		case "styleDeleted":