Handling multiple windows

[moonbat]

All my extensions have one Kryptonite - using multiple windows. Most people use tabbed browsing rather than separate windows, but when they do, there are problems with the state of overlaid UI elements.
In all my extensions, I use namespaces, <extension name>.namespace.
A common javascript module for utility functions, and separate sub-namespaces for the overlay window, options dialog etc.
For example, my browser overlay script -

Code: Select all

if ("undefined" == typeof(PureURL)) {
	var PureURL = {};
}
const puPrefBranch = PureURL.Common.prefBranch;
PureURL.Launch={
// All browser window related stuff goes here.
}
window.addEventListener("load", function(){PureURL.Launch.init();},false);
window.addEventListener("unload", PureURL.Launch.cleanup,false);

Part of my code involves enabling/disabling toolbar buttons when they are not applicable (in PMPlayer, when the current page has no video on it, in PureURL when the current page isn't a webpage). If you now open a new window and close it, the button on the previous window stops responding to events. If I null the button in the cleanup function, then it throws errors in the previous window.

Why does this happen? Shouldn't there be a new copy of the PureURL.Launch namespace when a new window opens, and only that go out of scope when the window closes? Otherwise what is the difference between using a namespace like this and using a .jsm module (which is expected to remain loaded throughout the existence of the browser process and not dependent on individual windows?)

[Moonchild]

You probably want an init safeguard; add a property on your prototype that you set when init is completed so you don't get multiple inits. (e.g. add an isInitialized boolean)
Alternatively don't attach the extension init globals to window loading but to browser startup instead and only set per-window changes in the onload handler?

I haven't looked at your code but there are common ways around this kind of duplicate scripting causing issues, depending on how exactly it's used.

[JustOff]

Shouldn't there be a new copy of the PureURL.Launch namespace when a new window opens

The problem is that you are creating PureURL object from common.jsm, not from init.js, which makes PureURL shared across all windows.

The simplest way to fix this is to move import

Code: Select all

Components.utils.import("chrome://pureurl4pm/content/common.jsm");

below the PureURL init block

Code: Select all

if ("undefined" == typeof(PureURL)) {
	var PureURL = {};
}

[moonbat]

don't attach the extension init globals to window loading but to browser startup instead

How does one event-handle that?

The simplest way to fix this is to move import

Didn't help
I tested it by setting PureURL.Launch (namespace for all functions in the window) to null in the close window handler, and it got applied to the one in the existing window as well.

[JustOff]

Well, I looked a little more carefully and looks like you are a bit confused with namespaces.

How about this:

Code: Select all

diff --git a/src/content/common.jsm b/src/content/common.jsm
index e5b3117..f2f94db 100644
--- a/src/content/common.jsm
+++ b/src/content/common.jsm
@@ -1,10 +1,7 @@
-var EXPORTED_SYMBOLS = ["PureURL","Cc","Ci","Cu","PureURLPrefListener"]
+var EXPORTED_SYMBOLS = ["Common", "PureURLPrefListener"]
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 Cu.import("resource://gre/modules/Services.jsm");
-if ("undefined" == typeof(PureURL)) {
-	var PureURL = {};
-}
-PureURL.Common={
+Common={
 		pureURLObserver : null,
 		logoutput : false,
 		linkcount : 0,
@@ -16,13 +13,13 @@ PureURL.Common={
 		 * Toggle console logging.
 		 */
 		setLogging:function(flag){
-			PureURL.Common.logoutput = flag;
+			Common.logoutput = flag;
 		},
 		/**
 		 * Log output to toolkit error console if enabled.
 		 */ 
 		printlog:function(item) {
-			if(PureURL.Common.logoutput==true) {
+			if(Common.logoutput==true) {
 				Services.console.logStringMessage("[PureURL:]"+item);
 			}
 		},
@@ -171,9 +168,9 @@ PureURL.Common={
 				var response = url.getResponseHeader('Location');
 				if(Math.round(url.responseStatus/100) == 3 && response){
 					if(el != response) {
-						PureURL.Common.printlog("Resolved "+el+" to "+response);
+						Common.printlog("Resolved "+el+" to "+response);
 					}
-					return el==response ? response : PureURL.Common.unshortenLink(response);
+					return el==response ? response : Common.unshortenLink(response);
 				}
 			}catch(e){
 				return el;
@@ -197,21 +194,21 @@ PureURL.Common={
 				if(!host)
 					continue;
 				var qpos = href.indexOf('?');
-				if(qpos===-1||PureURL.Common.checkedLinks.hasOwnProperty(href)){
-					if(PureURL.Common.prefBranch.getBoolPref("expandlinks") 
-							&& PureURL.Common.shorteners.has(host)){
-						el.href = PureURL.Common.unshortenLink(el);
+				if(qpos===-1||Common.checkedLinks.hasOwnProperty(href)){
+					if(Common.prefBranch.getBoolPref("expandlinks") 
+							&& Common.shorteners.has(host)){
+						el.href = Common.unshortenLink(el);
 						if(orig != el.href){
-							PureURL.Common.printlog("Original link: "+orig
+							Common.printlog("Original link: "+orig
 							                        +"\nExpanded link: "+el.href);
 							el.title=el.href;
-							PureURL.Common.expandcount++;
+							Common.expandcount++;
 						}
 					}
 					continue;
 				}
 				var args = href.substring(qpos+1, href.length);
-				var regexp = PureURL.Common.gfields.getRegExp(host);
+				var regexp = Common.gfields.getRegExp(host);
 				if(regexp) {
 					var cleaned = args.replace(regexp, '').replace(/^[&]+/i, '');
 					if(args!==cleaned) {
@@ -219,11 +216,11 @@ PureURL.Common={
 						if(cleaned)
 							href += '?'+cleaned;
 						el.href = href;
-						PureURL.Common.linkcount++;
-						PureURL.Common.printlog("Original link: "+orig+"\nCleaned link:"+href);
+						Common.linkcount++;
+						Common.printlog("Original link: "+orig+"\nCleaned link:"+href);
 					}
 				}
-				PureURL.Common.checkedLinks[el.href] = true;
+				Common.checkedLinks[el.href] = true;
 			}
 			return root;
 		}
@@ -233,12 +230,12 @@ function pureURLHttpObserver() {
 pureURLHttpObserver.prototype = {
 		observe : function(aSubject,aTopic,aData){
 			if ("http-on-modify-request" == aTopic && 
-					PureURL.Common.prefBranch.getBoolPref("request_hook_enabled",true)) {
+					Common.prefBranch.getBoolPref("request_hook_enabled",true)) {
 				aSubject.QueryInterface(Ci.nsIHttpChannel);
 				url = aSubject.URI.spec;
 				path = aSubject.URI.path;
 				qpos = path.indexOf('?');
-				regexp = PureURL.Common.gfields.getRegExp(aSubject.URI.host);
+				regexp = Common.gfields.getRegExp(aSubject.URI.host);
 				if (regexp !== null && qpos > -1) {
 					var args = path.substring(qpos + 1, path.length);
 					var cleaned = args.replace(regexp, '').replace(/^[&]+/i, '');
@@ -247,7 +244,7 @@ pureURLHttpObserver.prototype = {
 						if (cleaned) path += '?' + cleaned;
 						try {
 							aSubject.URI.path = path;
-							PureURL.Common.printlog("Original request: "+url+"\nCleaned request:"+aSubject.URI.spec);
+							Common.printlog("Original request: "+url+"\nCleaned request:"+aSubject.URI.spec);
 						} catch (e) {
 							Cu.reportError("[PureURL error:] URI= "+aSubject.URI.spec+"\nmodified path="+path+"\n"+e);
 						}
@@ -256,7 +253,7 @@ pureURLHttpObserver.prototype = {
 			}
 		}
 }
-PureURL.Common.pureURLObserver = new pureURLHttpObserver();
+Common.pureURLObserver = new pureURLHttpObserver();
 function PureURLPrefListener(branch_name, callback) {
 	// Keeping a reference to the observed preference branch or it will get
 	// garbage collected.
diff --git a/src/content/init.js b/src/content/init.js
index 14dd206..3daa441 100644
--- a/src/content/init.js
+++ b/src/content/init.js
@@ -1,9 +1,9 @@
-Components.utils.import("chrome://pureurl4pm/content/common.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 if ("undefined" == typeof(PureURL)) {
 	var PureURL = {};
 }
+Components.utils.import("chrome://pureurl4pm/content/common.jsm", PureURL);
 const puPrefBranch = PureURL.Common.prefBranch;
 PureURL.Launch={
 		btnchkIntercept:null,
@@ -70,7 +70,7 @@ PureURL.Launch={
 		/**
 		 * Listens for preference changes and applies them to the UI.
 		 */
-		prefListener : new PureURLPrefListener("extensions.pure-url@palemoon.",function(branch,name){
+		prefListener : new PureURL.PureURLPrefListener("extensions.pure-url@palemoon.",function(branch,name){
 			switch(name){
 			case("request_hook_enabled"):
 				var intercept = puPrefBranch.getBoolPref("request_hook_enabled",true);

[moonbat]

I'll try this out. I wanted to keep everything under a sub namespace of PureURL so that Common doesn't conflict with any other extension having a similar name. Or maybe I should name it PureURLCommon or something.

[JustOff]

I wanted to keep everything under a sub namespace of PureURL so that Common doesn't conflict with any other extension having a similar name.

And it will be right there because of:

Code: Select all

-Components.utils.import("chrome://pureurl4pm/content/common.jsm");
+Components.utils.import("chrome://pureurl4pm/content/common.jsm", PureURL);

[moonbat]

Thanks JustOff!