Fixing issue #98 (infinite loop when location hash set empty)

Added tests and fixed the issue. Closes #98
angular · Nov 1, 2010 · 99f2505 · 99f2505
1 parent ba5f8ee
commit 99f2505
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 8 deletions.
diff --git a/src/services.js b/src/services.js
@@ -16,20 +16,24 @@ angularServiceInject("$document", function(window){
 angularServiceInject("$location", function(browser) {
   var scope = this,
       location = {toString:toString, update:update, updateHash: updateHash},
-      lastLocationHref = browser.getUrl(),
+      lastBrowserUrl = browser.getUrl(),
+      lastLocationHref,
       lastLocationHash;
 
-  browser.addPollFn(function(){
-    if (lastLocationHref !== browser.getUrl()) {
-      update(lastLocationHref = browser.getUrl());
+  browser.addPollFn(function() {
+    if (lastBrowserUrl != browser.getUrl()) {
+      update(lastBrowserUrl = browser.getUrl());
+      lastLocationHref = location.href;
+      lastLocationHash = location.hash;
       scope.$eval();
     }
   });
 
   this.$onEval(PRIORITY_FIRST, updateBrowser);
   this.$onEval(PRIORITY_LAST, updateBrowser);
 
-  update(lastLocationHref);
+  update(lastBrowserUrl);
+  lastLocationHref = location.href;
   lastLocationHash = location.hash;
 
   return location;
@@ -137,8 +141,9 @@ angularServiceInject("$location", function(browser) {
   function updateBrowser() {
     updateLocation();
 
-    if (location.href != lastLocationHref) {
-      browser.setUrl(lastLocationHref = location.href);
+    if (location.href != lastLocationHref) {    	
+      browser.setUrl(lastBrowserUrl = location.href);
+      lastLocationHref = location.href;
       lastLocationHash = location.hash;
     }
   }
@@ -180,7 +185,7 @@ angularServiceInject("$location", function(browser) {
     var match = URL_MATCH.exec(href);
 
     if (match) {
-      loc.href = href.replace('#$', '');
+      loc.href = href.replace(/#$/, '');
       loc.protocol = match[1];
       loc.host = match[3] || '';
       loc.port = match[5] || DEFAULT_PORTS[loc.protocol] || _null;

diff --git a/test/servicesSpec.js b/test/servicesSpec.js
@@ -207,6 +207,18 @@ describe("service", function(){
       expect(scope.$location.hashSearch).toEqual({a: 'b'});
       expect(scope.$location.hashPath).toEqual('path');
     });
+
+    it('should remove # if hash is empty', function() {
+      scope.$location.update('http://www.angularjs.org/index.php#');
+      expect(scope.$location.href).toEqual('http://www.angularjs.org/index.php');
+    });
+
+    it('should not change browser\'s url with empty hash', function() {
+      $browser.setUrl('http://www.angularjs.org/index.php#');
+      spyOn($browser, 'setUrl');
+      $browser.poll();
+      expect($browser.setUrl).not.toHaveBeenCalled();
+    });
   });
 
   describe("$invalidWidgets", function(){