$location service should utilize onhashchange events instead of polling

9 files changed, 187 insertions, 49 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e091ec1d..9f04983b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,8 @@
 ### Performance
 - $location and $cookies services are now lazily initialized to avoid the polling overhead when
   not needed.
+- $location service now listens for `onhashchange` events (if supported by browser) instead of
+  constant polling.
 
 ### Breaking changes
  - API for accessing registered services — `scope.$inject` — was renamed to
diff --git a/src/Angular.js b/src/Angular.js
index 8b61970f..8ada7be6 100644
--- a/src/Angular.js
+++ b/src/Angular.js
@@ -271,7 +271,7 @@ function jqLiteWrap(element) {
       var div = document.createElement('div');
       div.innerHTML = element;
       element = new JQLite(div.childNodes);
-    } else if (!(element instanceof JQLite) && isElement(element)) {
+    } else if (!(element instanceof JQLite)) {
       element =  new JQLite(element);
     }
   }
diff --git a/src/AngularPublic.js b/src/AngularPublic.js
index 38325404..ab37a772 100644
--- a/src/AngularPublic.js
+++ b/src/AngularPublic.js
@@ -10,13 +10,8 @@ var browserSingleton;
  */
 angularService('$browser', function($log){
   if (!browserSingleton) {
-    browserSingleton = new Browser(
-        window.location,
-        jqLite(window.document),
-        jqLite(window.document.getElementsByTagName('head')[0]),
-        XHR,
-        $log,
-        window.setTimeout);
+    browserSingleton = new Browser(window, jqLite(window.document), jqLite(window.document.body),
+                                   XHR, $log);
     var addPollFn = browserSingleton.addPollFn;
     browserSingleton.addPollFn = function(){
       browserSingleton.addPollFn = addPollFn;
diff --git a/src/Browser.js b/src/Browser.js
index 4ab92f10..c93f115c 100644
--- a/src/Browser.js
+++ b/src/Browser.js
@@ -8,8 +8,29 @@ var XHR = window.XMLHttpRequest || function () {
   throw new Error("This browser does not support XMLHttpRequest.");
 };
 
-function Browser(location, document, head, XHR, $log, setTimeout) {
-  var self = this;
+/**
+ * @private
+ * @name Browser
+ *
+ * @description
+ * Constructor for the object exposed as $browser service.
+ *
+ * This object has two goals:
+ *
+ * - hide all the global state in the browser caused by the window object
+ * - abstract away all the browser specific features and inconsistencies
+ *
+ * @param {object} window The global window object.
+ * @param {object} document jQuery wrapped document.
+ * @param {object} body jQuery wrapped document.body.
+ * @param {function()} XHR XMLHttpRequest constructor.
+ * @param {object} $log console.log or an object with the same interface.
+ */
+function Browser(window, document, body, XHR, $log) {
+  var self = this,
+      location = window.location,
+      setTimeout = window.setTimeout;
+
   self.isMock = false;
 
   //////////////////////////////////////////////////////////////
@@ -70,7 +91,7 @@ function Browser(location, document, head, XHR, $log, setTimeout) {
         window[callbackId] = _undefined;
         callback(200, data);
       };
-      head.append(script);
+      body.append(script);
     } else {
       var xhr = new XHR();
       xhr.open(method, url, true);
@@ -195,6 +216,39 @@ function Browser(location, document, head, XHR, $log, setTimeout) {
     return location.href;
   };
 
+
+  /**
+   * @workInProgress
+   * @ngdoc method
+   * @name angular.service.$browser#onHashChange
+   * @methodOf angular.service.$browser
+   *
+   * @description
+   * Detects if browser support onhashchange events and register a listener otherwise registers
+   * $browser poller. The `listener` will then get called when the hash changes.
+   *
+   * The listener gets called with either HashChangeEvent object or simple object that also contains
+   * `oldURL` and `newURL` properties.
+   *
+   * NOTE: this is a api is intended for sole use by $location service. Please use
+   * {@link angular.service.$location $location service} to monitor hash changes in angular apps.
+   *
+   * @param {function(event)} listener Listener function to be called when url hash changes.
+   */
+  self.onHashChange = function(listener) {
+    if ('onhashchange' in window) {
+      jqLite(window).bind('hashchange', listener);
+    } else {
+      var lastBrowserUrl = self.getUrl();
+
+      self.addPollFn(function() {
+        if (lastBrowserUrl != self.getUrl()) {
+          listener();
+        }
+      });
+    }
+  }
+
   //////////////////////////////////////////////////////////////
   // Cookies API
   //////////////////////////////////////////////////////////////
@@ -338,7 +392,7 @@ function Browser(location, document, head, XHR, $log, setTimeout) {
     link.attr('rel', 'stylesheet');
     link.attr('type', 'text/css');
     link.attr('href', url);
-    head.append(link);
+    body.append(link);
   };
 
 
@@ -359,6 +413,6 @@ function Browser(location, document, head, XHR, $log, setTimeout) {
     script.attr('type', 'text/javascript');
     script.attr('src', url);
     if (dom_id) script.attr('id', dom_id);
-    head.append(script);
+    body.append(script);
   };
 }
diff --git a/src/jqLite.js b/src/jqLite.js
index 0d96d6e4..1bc966eb 100644
--- a/src/jqLite.js
+++ b/src/jqLite.js
@@ -47,14 +47,14 @@ function getStyle(element) {
 }
 
 function JQLite(element) {
-  if (isElement(element)) {
-    this[0] = element;
-    this.length = 1;
-  } else if (isDefined(element.length) && element.item) {
+  if (!isElement(element) && isDefined(element.length) && element.item) {
     for(var i=0; i < element.length; i++) {
       this[i] = element[i];
     }
     this.length = element.length;
+  } else {
+    this[0] = element;
+    this.length = 1;
   }
 }
 
@@ -81,7 +81,7 @@ JQLite.prototype = {
   dealoc: function(){
     (function dealoc(element){
       jqClearData(element);
-      for ( var i = 0, children = element.childNodes; i < children.length; i++) {
+      for ( var i = 0, children = element.childNodes || []; i < children.length; i++) {
         dealoc(children[i]);
       }
     })(this[0]);
diff --git a/src/services.js b/src/services.js
index 0b983ffb..91bd226d 100644
--- a/src/services.js
+++ b/src/services.js
@@ -68,19 +68,17 @@ angularServiceInject("$document", function(window){
    <input type='text' name="$location.hash"/>
    <pre>$location = {{$location}}</pre>
  */
-angularServiceInject("$location", function(browser) {
+angularServiceInject("$location", function($browser) {
   var scope = this,
       location = {toString:toString, update:update, updateHash: updateHash},
-      lastBrowserUrl = browser.getUrl(),
+      lastBrowserUrl = $browser.getUrl(),
       lastLocationHref,
       lastLocationHash;
 
-  browser.addPollFn(function() {
-    if (lastBrowserUrl != browser.getUrl()) {
-      update(lastBrowserUrl = browser.getUrl());
-      updateLastLocation();
-      scope.$eval();
-    }
+  $browser.onHashChange(function() {
+    update(lastBrowserUrl = $browser.getUrl());
+    updateLastLocation();
+    scope.$eval();
   });
 
   this.$onEval(PRIORITY_FIRST, updateBrowser);
@@ -219,7 +217,7 @@ angularServiceInject("$location", function(browser) {
     updateLocation();
 
     if (location.href != lastLocationHref) {    	
-      browser.setUrl(lastBrowserUrl = location.href);
+      $browser.setUrl(lastBrowserUrl = location.href);
       updateLastLocation();
     }
   }
diff --git a/test/BrowserSpecs.js b/test/BrowserSpecs.js
index eb43e3c5..89fc14ed 100644
--- a/test/BrowserSpecs.js
+++ b/test/BrowserSpecs.js
@@ -1,6 +1,6 @@
 describe('browser', function(){
 
-  var browser, location, head, xhr, setTimeoutQueue;
+  var browser, fakeWindow, xhr, logs, scripts, setTimeoutQueue;
 
   function fakeSetTimeout(fn) {
     setTimeoutQueue.push(fn);
@@ -15,19 +15,31 @@ describe('browser', function(){
 
   beforeEach(function(){
     setTimeoutQueue = [];
-
-    location = {href:"http://server", hash:""};
-    head = {
-        scripts: [],
-        append: function(node){head.scripts.push(node);}
-    };
+    scripts = [];
     xhr = null;
-    browser = new Browser(location, jqLite(window.document), head, function(){
+    fakeWindow = {
+      location: {href:"http://server"},
+      setTimeout: fakeSetTimeout
+    }
+
+    var fakeBody = {append: function(node){scripts.push(node)}};
+
+    var fakeXhr = function(){
       xhr = this;
       this.open = noop;
       this.setRequestHeader = noop;
       this.send = noop;
-    }, undefined, fakeSetTimeout);
+    }
+
+    logs = {log:[], warn:[], info:[], error:[]};
+
+    var fakeLog = {log: function() { logs.log.push(slice.call(arguments)); },
+                   warn: function() { logs.warn.push(slice.call(arguments)); },
+                   info: function() { logs.info.push(slice.call(arguments)); },
+                   error: function() { logs.error.push(slice.call(arguments)); }};
+
+    browser = new Browser(fakeWindow, jqLite(window.document), fakeBody, fakeXhr,
+                          fakeLog);
   });
 
   it('should contain cookie cruncher', function() {
@@ -60,13 +72,13 @@ describe('browser', function(){
         browser.xhr('JSON', 'http://example.org/path?cb=JSON_CALLBACK', function(code, data){
           log += code + ':' + data + ';';
         });
-        expect(head.scripts.length).toEqual(1);
-        var url = head.scripts[0].src.split('?cb=');
+        expect(scripts.length).toEqual(1);
+        var url = scripts[0].src.split('?cb=');
         expect(url[0]).toEqual('http://example.org/path');
-        expect(typeof window[url[1]]).toEqual($function);
-        window[url[1]]('data');
+        expect(typeof fakeWindow[url[1]]).toEqual($function);
+        fakeWindow[url[1]]('data');
         expect(log).toEqual('200:data;');
-        expect(typeof window[url[1]]).toEqual('undefined');
+        expect(typeof fakeWindow[url[1]]).toEqual('undefined');
       });
     });
   });
@@ -107,16 +119,8 @@ describe('browser', function(){
       }
     }
 
-    var browser, log, logs;
-
     beforeEach(function() {
       deleteAllCookies();
-      logs = {log:[], warn:[], info:[], error:[]};
-      log = {log: function() { logs.log.push(slice.call(arguments)); },
-             warn: function() { logs.warn.push(slice.call(arguments)); },
-             info: function() { logs.info.push(slice.call(arguments)); },
-             error: function() { logs.error.push(slice.call(arguments)); }};
-      browser = new Browser({}, jqLite(document), undefined, XHR, log);
       expect(document.cookie).toEqual('');
     });
 
@@ -334,4 +338,62 @@ describe('browser', function(){
       expect(returnedFn).toBe(fn);
     });
   });
+
+
+  describe('url api', function() {
+    it('should use $browser poller to detect url changes when onhashchange event is unsupported',
+        function() {
+
+      fakeWindow = {location: {href:"http://server"}};
+
+      browser = new Browser(fakeWindow, {}, {});
+
+      var events = [];
+
+      browser.onHashChange(function() {
+        events.push('x');
+      });
+
+      fakeWindow.location.href = "http://server/#newHash";
+      expect(events).toEqual([]);
+      browser.poll();
+      expect(events).toEqual(['x']);
+    });
+
+
+    it('should use onhashchange events to detect url changes when supported by browser',
+        function() {
+
+      var onHashChngListener;
+
+      fakeWindow = {location: {href:"http://server"},
+                    addEventListener: function(type, listener) {
+                      expect(type).toEqual('hashchange');
+                      onHashChngListener = listener;
+                    },
+                    removeEventListener: angular.noop
+                   };
+      fakeWindow.onhashchange = true;
+
+      browser = new Browser(fakeWindow, {}, {});
+
+      var events = [],
+          event = {type: "hashchange"}
+
+      browser.onHashChange(function(e) {
+        events.push(e);
+      });
+
+      expect(events).toEqual([]);
+      onHashChngListener(event);
+
+      expect(events.length).toBe(1);
+      expect(events[0].originalEvent || events[0]).toBe(event); // please jQuery and jqLite
+
+      // clean up the jqLite cache so that the global afterEach doesn't complain
+      if (!jQuery) {
+        jqLite(fakeWindow).dealoc();
+      }
+    });
+  });
 });
diff --git a/test/angular-mocks.js b/test/angular-mocks.js
index fd53a189..5a4e1de5 100644
--- a/test/angular-mocks.js
+++ b/test/angular-mocks.js
@@ -63,8 +63,23 @@ function MockBrowser() {
 
   this.isMock = true;
   self.url = "http://server";
+  self.lastUrl = self.url; // used by url polling fn
   self.pollFns = [];
 
+
+  // register url polling fn
+
+  self.onHashChange = function(listener) {
+    self.pollFns.push(
+      function() {
+        if (self.lastUrl != self.url) {
+          listener();
+        }
+      }
+    );
+  };
+
+
   self.xhr = function(method, url, data, callback) {
     if (angular.isFunction(data)) {
       callback = data;
diff --git a/test/servicesSpec.js b/test/servicesSpec.js
index b2cac224..6df83beb 100644
--- a/test/servicesSpec.js
+++ b/test/servicesSpec.js
@@ -128,6 +128,18 @@ describe("service", function(){
       expect($location.hashSearch).toEqual({key: 'value', flag: true, key2: ''});
     });
 
+
+    it('should update location when browser url changed', function() {
+      var origUrl = $location.href;
+      expect(origUrl).toEqual($browser.getUrl());
+
+      var newUrl = 'http://somenew/url#foo';
+      $browser.setUrl(newUrl);
+      $browser.poll();
+      expect($location.href).toEqual(newUrl);
+    });
+
+
     it('toString() should return actual representation', function() {
       var href = 'http://host:123/p/a/t/h.html?query=value#path?key=value&flag&key2=';
       $location.update(href);