changeset 139:8763c90ce556

better support for feeds that require authentication; we now display a custom authentication dialog that hopefully makes more sense when it appears out of the blue; we also use it when subscribing the user to the feed, and it includes UI for the user to request that their credentials be remembered; finally, we use remembered credentials automatically across sessions instead of prompting at least once per session
author Myk Melez <myk@mozilla.org>
date Mon, 07 Jul 2008 01:22:44 -0700
parents 58b04d16257c
children 70f46dff93a9
files extension/content/subscribe.css extension/content/subscribe.js extension/content/subscribe.xul extension/modules/feed.js
diffstat 4 files changed, 219 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/extension/content/subscribe.css	Sun Jul 06 00:05:45 2008 -0700
+++ b/extension/content/subscribe.css	Mon Jul 07 01:22:44 2008 -0700
@@ -38,3 +38,7 @@
 .statusBox[status="complete"] > .statusIcon {
   list-style-image: url("chrome://snowl/content/icons/tick.png");
 }
+
+.statusBox[status="error"] > .statusIcon {
+  list-style-image: url("chrome://global/skin/icons/error-16.png");
+}
--- a/extension/content/subscribe.js	Sun Jul 06 00:05:45 2008 -0700
+++ b/extension/content/subscribe.js	Mon Jul 07 01:22:44 2008 -0700
@@ -3,12 +3,15 @@
 const Cr = Components.results;
 const Cu = Components.utils;
 
-// Generic modules
+// modules that come with Firefox
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+// modules that should come with Firefox
 Cu.import("resource://snowl/modules/Observers.js");
 Cu.import("resource://snowl/modules/URI.js");
 Cu.import("resource://snowl/modules/log4moz.js");
 
-// Snowl modules
+// Snowl-specific modules
 Cu.import("resource://snowl/modules/service.js");
 Cu.import("resource://snowl/modules/datastore.js");
 Cu.import("resource://snowl/modules/feed.js");
@@ -16,26 +19,23 @@
 window.addEventListener("load", function() { Subscriber.init() }, false);
 
 function SubscriptionListener(subject, topic, data) {
+  if (subject != Subscriber.feed)
+    return;
+
   switch(topic) {
     case "snowl:subscribe:connect:start":
       document.getElementById("connectingBox").disabled = false;
       document.getElementById("connectingBox").setAttribute("status", "active");
-      document.getElementById("authenticatingBox").disabled = true;
-      document.getElementById("authenticatingBox").removeAttribute("status");
       document.getElementById("gettingMessagesBox").disabled = true;
       document.getElementById("gettingMessagesBox").removeAttribute("status");
       document.getElementById("doneBox").disabled = true;
       document.getElementById("doneBox").removeAttribute("status");
       break;
     case "snowl:subscribe:connect:end":
-      document.getElementById("connectingBox").setAttribute("status", "complete");
-      break;
-    case "snowl:subscribe:authenticate:start":
-      document.getElementById("authenticatingBox").disabled = false;
-      document.getElementById("authenticatingBox").setAttribute("status", "active");
-      break;
-    case "snowl:subscribe:authenticate:end":
-      document.getElementById("authenticatingBox").setAttribute("status", "complete");
+      if (data < 200 || data > 299)
+        document.getElementById("connectingBox").setAttribute("status", "error");
+      else
+        document.getElementById("connectingBox").setAttribute("status", "complete");
       break;
     case "snowl:subscribe:get:start":
       document.getElementById("gettingMessagesBox").disabled = false;
@@ -61,8 +61,6 @@
   init: function() {
     Observers.add(SubscriptionListener, "snowl:subscribe:connect:start");
     Observers.add(SubscriptionListener, "snowl:subscribe:connect:end");
-    Observers.add(SubscriptionListener, "snowl:subscribe:authenticate:start");
-    Observers.add(SubscriptionListener, "snowl:subscribe:authenticate:end");
     Observers.add(SubscriptionListener, "snowl:subscribe:get:start");
     Observers.add(SubscriptionListener, "snowl:subscribe:get:progress");
     Observers.add(SubscriptionListener, "snowl:subscribe:get:end");
@@ -87,8 +85,6 @@
   destroy: function() {
     Observers.remove(SubscriptionListener, "snowl:subscribe:connect:start");
     Observers.remove(SubscriptionListener, "snowl:subscribe:connect:end");
-    Observers.remove(SubscriptionListener, "snowl:subscribe:authenticate:start");
-    Observers.remove(SubscriptionListener, "snowl:subscribe:authenticate:end");
     Observers.remove(SubscriptionListener, "snowl:subscribe:get:start");
     Observers.remove(SubscriptionListener, "snowl:subscribe:get:progress");
     Observers.remove(SubscriptionListener, "snowl:subscribe:get:end");
@@ -101,7 +97,7 @@
   doSubscribe: function() {
     let uri = URI.get(document.getElementById("snowlLocationTextbox").value);
     let feed = new SnowlFeed(null, null, uri);
-    feed.subscribe();
+    this._subscribe(feed);
   },
 
   doImportOPML: function() {
@@ -115,6 +111,7 @@
     if (rv != Ci.nsIFilePicker.returnOK)
       return;
 
+    // FIXME: use a file utility to open the file instead of XMLHttpRequest.
     let request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].
                   createInstance(Ci.nsIXMLHttpRequest);
     request.open("GET", fp.fileURL.spec, false);
@@ -141,12 +138,14 @@
     // If this outline represents a feed, subscribe the user to the feed.
     let uri = URI.get(aOutline.getAttribute("xmlUrl"));
     if (uri) {
-      // FIXME: make sure the user isn't already subscribed to the feed
-      // before subscribing her to it.
       let name = aOutline.getAttribute("title") || aOutline.getAttribute("text");
+      let feed = new SnowlFeed(null, name || "untitled", uri);
+
+      // XXX Should the next line be refactored into _subscribe?
       document.getElementById("sourceTitle").value = "Subscribing to " + (name || uri.spec);
-      var future = new Future();
-      new SnowlFeed(null, name || "untitled", uri).subscribe(future.fulfill);
+
+      let future = new Future();
+      this._subscribe(feed, future.fulfill);
       yield future.result();
     }
 
@@ -162,6 +161,24 @@
         yield this._importOutline(child);
       }
     }
+  }),
+
+  _subscribe: strand(function(feed, callback) {
+    // FIXME: make sure the user isn't already subscribed to the feed
+    // before subscribing them to it.
+
+    // Store a reference to the feed to which we are currently subscribing
+    // so the progress listener can filter out events for some other feed.
+    this.feed = feed;
+
+    let future = new Future();
+    feed.subscribe(future.fulfill);
+    yield future.result();
+
+    this.feed = null;
+
+    if (callback)
+      callback();
   })
 
 };
--- a/extension/content/subscribe.xul	Sun Jul 06 00:05:45 2008 -0700
+++ b/extension/content/subscribe.xul	Mon Jul 07 01:22:44 2008 -0700
@@ -14,25 +14,27 @@
   <vbox id="content">
     <label flex="1" value="Subscribe to Feed" class="header"/>
     <separator class="groove-thin"/>
-    <separator orient="horizontal"/>
+
+    <separator class="thin" orient="horizontal"/>
+
     <hbox align="center">
       <label control="snowlLocationTextbox" value="Location:"/>
       <textbox id="snowlLocationTextbox" flex="1"/>
       <button label="Subscribe" default="true" oncommand="Subscriber.doSubscribe(event)"/>
     </hbox>
 
-    <separator orient="horizontal"/>
+    <separator class="thin" orient="horizontal"/>
 
     <vbox>
       <label id="sourceTitle"/>
+
+      <separator class="thin" orient="horizontal"/>
+
       <hbox id="connectingBox" class="statusBox" align="center">
         <image id="connectingIcon" class="statusIcon"/>
         <label value="Connecting to feed" disabled="true"/>
       </hbox>
-      <hbox id="authenticatingBox" class="statusBox" align="center">
-        <image id="authenticatingIcon" class="statusIcon"/>
-        <label value="Authenticating (if necessary)" disabled="true"/>
-      </hbox>
+
       <hbox id="gettingMessagesBox" class="statusBox" align="center">
         <image id="gettingMessagesIcon" class="statusIcon"/>
         <label value="Getting stories" disabled="true"/>
@@ -43,7 +45,7 @@
       </hbox>
     </vbox>
 
-    <separator orient="horizontal"/>
+    <separator class="thin" orient="horizontal"/>
 
     <hbox>
       <button id="importOPMLButton" label="Import OPML..." oncommand="Subscriber.doImportOPML(event)"/>
--- a/extension/modules/feed.js	Sun Jul 06 00:05:45 2008 -0700
+++ b/extension/modules/feed.js	Mon Jul 07 01:22:44 2008 -0700
@@ -5,13 +5,18 @@
 const Cr = Components.results;
 const Cu = Components.utils;
 
+// modules that come with Firefox
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/ISO8601DateUtils.jsm");
 
+// modules that should come with Firefox
 Cu.import("resource://snowl/modules/log4moz.js");
-Cu.import("resource://snowl/modules/datastore.js");
+Cu.import("resource://snowl/modules/Observers.js");
 Cu.import("resource://snowl/modules/URI.js");
+
+// Snowl-specific modules
+Cu.import("resource://snowl/modules/datastore.js");
 Cu.import("resource://snowl/modules/source.js");
-Cu.import("resource://snowl/modules/Observers.js");
 
 // FIXME: factor this out into a common file.
 const PART_TYPE_CONTENT = 1;
@@ -45,6 +50,11 @@
 
   _log: Log4Moz.Service.getLogger("Snowl.Feed"),
 
+  // If we prompt the user to authenticate, and the user asks us to remember
+  // their password, we store the nsIAuthInformation in this property until
+  // the request succeeds, at which point we store it with the login manager.
+  _authInfo: null,
+
   // Observer Service
   get _obsSvc() {
     let obsSvc = Cc["@mozilla.org/observer-service;1"].
@@ -53,6 +63,73 @@
     return this._obsSvc;
   },
 
+  // nsISupports
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIAuthPrompt2]),
+
+  // nsIInterfaceRequestor
+
+  getInterface: function(iid) {
+    return this.QueryInterface(iid);
+  },
+
+  // nsIAuthPrompt2
+
+  _logins: null,
+  _loginIndex: 0,
+
+  promptAuth: function(channel, level, authInfo) {
+    // Check saved logins before prompting the user.  We get them
+    // from the login manager and try each in turn until one of them works
+    // or we run out of them.
+    if (!this._logins) {
+      let lm = Cc["@mozilla.org/login-manager;1"].getService(Ci.nsILoginManager);
+      // XXX Should we be using channel.URI.prePath in case the old URI
+      // redirects us to a new one at a different hostname?
+      this._logins = lm.findLogins({}, this.machineURI.prePath, null, authInfo.realm);
+    }
+
+    let login = this._logins[this._loginIndex];
+    if (login) {
+      authInfo.username = login.username;
+      authInfo.password = login.password;
+      ++this._loginIndex;
+      return true;
+    }
+
+    // If we've made it this far, none of the saved logins worked, so we prompt
+    // the user to provide one.
+    let args = Cc["@mozilla.org/supports-array;1"].createInstance(Ci.nsISupportsArray);
+    args.AppendElement({ wrappedJSObject: this });
+    args.AppendElement(authInfo);
+
+    // |result| is how the dialog passes information back to us.  It sets two
+    // properties on the object: |proceed|, which we return from this function,
+    // and which determines whether or not authentication can proceed using
+    // the values entered by the user; and |remember|, which determines whether
+    // or not we save the user's login with the login manager once the request
+    // succeeds.
+    let result = {};
+    args.AppendElement({ wrappedJSObject: result });
+
+    let ww = Cc["@mozilla.org/embedcomp/window-watcher;1"].getService(Ci.nsIWindowWatcher);
+    ww.openWindow(null,
+                  // XXX Should we use commonDialog.xul?
+                  "chrome://snowl/content/login.xul",
+                  null,
+                  "chrome,centerscreen,dialog,modal",
+                  args);
+
+    if (result.remember)
+      this._authInfo = authInfo;
+
+    return result.proceed;
+  },
+
+  asyncPromptAuth: function() {
+    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
+  },
+
   refresh: function() {
     let request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance();
 
@@ -68,15 +145,34 @@
     request.overrideMimeType("text/plain");
 
     request.open("GET", this.machineURI.spec, true);
+
+    // Register a listener for notification callbacks so we handle authentication.
+    request.channel.notificationCallbacks = this;
+
     request.send(null);
   },
 
   onRefreshLoad: function(aEvent) {
     let request = aEvent.target;
 
+    // If the request failed, let the error handler handle it.
+    // XXX Do we need this?  Don't such failures call the error handler directly?
+    if (request.status < 200 || request.status > 299) {
+      this.onRefreshError(aEvent);
+      return;
+    }
+
     // XXX What's the right way to handle this?
-    if (request.responseText.length == 0)
-      throw("feed contains no data");
+    if (request.responseText.length == 0) {
+      this.onRefreshError(aEvent);
+      return;
+    }
+
+    // _authInfo only gets set if we prompted the user to authenticate
+    // and the user checked the "remember password" box.  Since we're here,
+    // it means the request succeeded, so we save the login.
+    if (this._authInfo)
+      this._saveLogin();
 
     let parser = Cc["@mozilla.org/feed-processor;1"].
                  createInstance(Ci.nsIFeedProcessor);
@@ -85,8 +181,8 @@
   },
 
   onRefreshError: function(aEvent) {
-    this._log.error("onRefreshError: " + aEvent.target.status + " " +
-                    aEvent.target.statusText + " " + aEvent.target.responseText.length);
+    let request = aEvent.target;
+    this._log.error("onRefreshError: " + request.status + " (" + request.statusText + ")");
   },
 
   onRefreshResult: function(aResult) {
@@ -376,20 +472,36 @@
     request.overrideMimeType("text/plain");
 
     request.open("GET", this.machineURI.spec, true);
+
+    // Register a listener for notification callbacks so we handle authentication.
+    request.channel.notificationCallbacks = this;
+
     request.send(null);
   },
 
   onSubscribeLoad: function(aEvent) {
-    Observers.notify(this, "snowl:subscribe:connect:end", null);
+    let request = aEvent.target;
 
-    let request = aEvent.target;
+    // If the request failed, let the error handler handle it.
+    // XXX Do we need this?  Don't such failures call the error handler directly?
+    if (request.status < 200 || request.status > 299) {
+      this.onSubscribeError(aEvent);
+      return;
+    }
 
     // XXX What's the right way to handle this?
-    if (request.responseText.length == 0)
-      throw("feed contains no data");
+    if (request.responseText.length == 0) {
+      this.onRefreshError(aEvent);
+      return;
+    }
 
-    Observers.notify(this, "snowl:subscribe:authenticate:start", null);
-    Observers.notify(this, "snowl:subscribe:authenticate:end", null);
+    Observers.notify(this, "snowl:subscribe:connect:end", request.status);
+
+    // _authInfo only gets set if we prompted the user to authenticate
+    // and the user checked the "remember password" box.  Since we're here,
+    // it means the request succeeded, so we save the login.
+    if (this._authInfo)
+      this._saveLogin();
 
     let parser = Cc["@mozilla.org/feed-processor;1"].
                  createInstance(Ci.nsIFeedProcessor);
@@ -398,8 +510,11 @@
   },
 
   onSubscribeError: function(aEvent) {
-    this._log.error("onSubscribeError: " + aEvent.target.status + " " +
-                    aEvent.target.statusText + " " + aEvent.target.responseText.length);
+    let request = aEvent.target;
+    this._log.error("onSubscribeError: " + request.status + " (" + request.statusText + ")");
+    Observers.notify(this, "snowl:subscribe:connect:end", request.status);
+    if (this._subscribeCallback)
+      this._subscribeCallback();
   },
 
   onSubscribeResult: function(aResult) {
@@ -441,6 +556,45 @@
       if (this._subscribeCallback)
         this._subscribeCallback();
     }
+  },
+
+  _saveLogin: function() {
+    let lm = Cc["@mozilla.org/login-manager;1"].getService(Ci.nsILoginManager);
+
+    // Create a new login with the auth information we obtained from the user.
+    let LoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
+                                               Ci.nsILoginInfo,
+                                               "init");
+    // XXX Should we be using channel.URI.prePath in case the old URI
+    // redirects us to a new one at a different hostname?
+    let newLogin = new LoginInfo(this.machineURI.prePath,
+                                 null,
+                                 this._authInfo.realm,
+                                 this._authInfo.username,
+                                 this._authInfo.password,
+                                 "",
+                                 "");
+
+    // Get existing logins that have the same hostname and realm.
+    let logins = lm.findLogins({}, this.machineURI.prePath, null, this._authInfo.realm);
+
+    // Try to figure out if we should replace one of the existing logins.
+    // If there's only one existing login, we replace it.  Otherwise, if
+    // there's a login with the same username, we replace that.  Otherwise,
+    // we add the new login instead of replacing an existing one.
+    let oldLogin;
+    if (logins.length == 1)
+      oldLogin = logins[0];
+    else if (logins.length > 1)
+      oldLogin = logins.filter(function(v) v.username == this._authInfo.username)[0];
+
+    if (oldLogin)
+      lm.modifyLogin(oldLogin, newLogin);
+    else
+      lm.addLogin(newLogin);
+
+    // Now that we've saved the login, we don't need the auth info anymore.
+    this._authInfo = null;
   }
 
 };