357472 - cookies can be added and rejected before onLocationChange() is called with progress listener

Status: RESOLVED WONTFIX

(Core :: Networking, defect)

Description

[sdb]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061003 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061003 Firefox/2.0

It turns out that cookies can be added and rejected before onLocationChange() is called. They can also be added and rejected after the page has finished loading. As a result it is nearly impossible to keep track of which cookies were added or rejected for the current site. I would normally expect onLocationChange() to be called before any cookies were sent to the browser. That would allow you to keep track of which cookies belonged to the current site by storing them in an array and clearing that array each time onLocationChange() was called. Unfortunately since cookies can be rejected for the current site before onLocationChange() is called then by clearing the array you would be losing any cookies rejected before onLocationChange() was called. I have also tried using onStateChange() to determine when all of the cookies have been added or rejected but it is also possible for a cookie to be rejected after the page has finished loading. Is this a bug or is this normal behavior for the progress listener? I have listed some code below which you can use to test this issue. Just set your homepage to circuitcity.com and disable/block all cookies. Then insert the code below into the main overlay file of one of your extensins.  Then close and restart your browser.  You will notice that 'cookie rejected' appears before 'location changed' when the page starts to load. And after the page has finished loading you will notice that 'cookie rejected' appears after 'page loaded'.  Any ideas how to get around this issue?

Reproducible: Always

Steps to Reproduce:
window.addEventListener('load',testProgressObserver,false);

function testProgressObserver() {
gBrowser.addProgressListener(testingProgress);
testingObserver.init();
}

var testingObserver = {

	showalert: 0,

	init: function() {
		var os = this.getObserver();
		os.addObserver(this, "cookie-changed", false);
		os.addObserver(this, "cookie-rejected", false);
	},

	observe: function(aCookie, aTopic, aData) {

		if (aTopic=='cookie-rejected') {
			if (this.showalert==0) {
				alert('cookie rejected');
				++this.showalert;
			}
		}

		if (aCookie instanceof Components.interfaces.nsICookie) {
			if (aData=="changed" || aData=="added") {
				if (this.showalert==0) {
					alert('cookie added');
					++this.showalert;
				}
			}
		}
	},

	getObserver: function() {
		return Components.classes["@mozilla.org/observer-service;1"].
		getService(Components.interfaces.nsIObserverService);
	}
};

var testingProgress = {

	QueryInterface: function(aIID) {
		if (aIID.equals(Components.interfaces.nsIWebProgressListener)   ||
		    aIID.equals(Components.interfaces.nsISupportsWeakReference) ||
		    aIID.equals(Components.interfaces.nsISupports))
      			return this;
		throw Components.results.NS_NOINTERFACE;
	},

	onLinkIconAvailable: function(a) { },
	onLocationChange: function(a,b,c) {
		alert('location changed');
	},
	onProgressChange: function(a,b,c,d,e,f) { },
	onSecurityChange: function(a,b,c) { },
	onStateChange: function(a,b,c,d) {
		if (!a.isLoadingDocument) {
			testingObserver.showalert=0;
			alert('page loaded');
		}
	},
	onStatusChange: function(a,b,c,d) { }
};
Actual Results:  
cookies are rejected before onlocationchange and after the page has finished loading.

Expected Results:  
The onlocationchange event should be triggered before any cookie notifications are sent to the observer.  And the onstatechange isLoadingDocument property should not be set to false until after all of the cookie notifications have been sent to the observer.  Otherwise the observer will continue to receive cookie-rejected notifications after the page has finished loading.

Due to this bug there is no current way to distinguish which cookies belong to which webpage.  All cookies should be added or rejected between the onlocationchange event and the final onstatechange event when the page has finished loading.

Comment 1

[sdb]

If you have blocked all cookies the url below will initiate 34 cookie-rejected notifications to be sent to the observer.  One of those 34 notifications occurs before the onlocationchange event.

If you allow all cookies the url below will initiate 3 cookie-changed notifications to be sent to the observer.  Two of those 3 notifications occur before the onlocationchange event.

I've tried using the getBrowser().currentURI.spec property as well as window.content.location.href property and it appears that cookie notifications can be sent to the observer before either of those properties are changed.

An easy way to test this issue is to create an array in the progress listener object and clear the array each time onlocationchange is called.  Then push the nsIURI object into the array for cookie-rejected notifications and push the nsICookie object into the array for cookie-changed notifications.  Then create a temporary menuitem with an oncommand event that displays an alert box with the length of the array.

http://circuitcity.com/ccd/home.do?JSESSIONID=FGJXTLk5LN2FT0fVskzJL1Ksq0pYkC!-655730351!1162250551613

observe: function(aCookie, aTopic, aData) {
   if (aTopic=='cookie-rejected') {
      testingProgress.rejected.push(aCookie);
   }

   if (aTopic=='cookie-changed') {
      testingProgress.changed.push(aCookie);
   }
},

var testingProgress = {
   blocked: [],
   changed: [],
   onLocationChange: function(a,b,c) {
      this.blocked = [];
      this.changed = [];
   },

<menuitem id="cookie-test1"
label="Show Length"
oncommand="alert(testingProgress.changed.length+'\n'+testingProgress.rejected.length);"/>

<menuitem id="cookie-test2"
label="Reset arrays"
oncommand="testingProgress.changed = []; testingProgress.rejected = [];"/>

Comment 2

[sdb]

I was able to solve this problem by using the StartDocumentLoad notification topic.  Instead of clearing the array each time onLocationChange() was called it is now cleared each time the StartDocumentLoad notification is sent to the observer.

os.addObserver(this, "StartDocumentLoad", false);

I would recommend a code change for future releases.  Each time the StartDocumentLoad notification is sent to the observer the gBrowser.currentURI property should be changed to the new location and the onLocationChange() event should be triggered.  That would eliminate any cookies from being added or rejected before the onLocationChange() event occurred.