fix($location): avoid unnecessary $locationChange* events due to empty hash

Fixes #16632

Author: gkalpak

src/ng/browser.js

@@ -1,5 +1,5 @@
 'use strict';
-/* global getHash: true, stripHash: false */
+/* global getHash: true, stripHash: false, trimEmptyHash: false */
 
 function getHash(url) {
   var index = url.indexOf('#');
@@ -143,7 +143,7 @@ function Browser(window, document, $log, $sniffer, $$taskTrackerFactory) {
       // - pendingLocation is needed as browsers don't allow to read out
       //   the new location.href if a reload happened or if there is a bug like in iOS 9 (see
       //   https://openradar.appspot.com/22186109).
-      return pendingLocation || location.href;
+      return pendingLocation || trimEmptyHash(location.href);
     }
   };
 

src/ng/location.js

@@ -1,5 +1,5 @@
 'use strict';
-/* global stripHash: true */
+/* global stripHash: true, trimEmptyHash: true */
 
 var PATH_MATCH = /^([^?#]*)(\?([^#]*))?(#(.*))?$/,
     DEFAULT_PORTS = {'http': 80, 'https': 443, 'ftp': 21};

test/ng/browserSpecs.js

@@ -404,6 +404,14 @@ describe('browser', function() {
       expect(browser.url()).toEqual('https://another.com');
     });
 
+    it('should strip an empty hash fragment', function() {
+      fakeWindow.location.href = 'http://test.com#';
+      expect(browser.url()).toEqual('http://test.com');
+
+      fakeWindow.location.href = 'https://another.com#foo';
+      expect(browser.url()).toEqual('https://another.com#foo');
+    });
+
     it('should use history.pushState when available', function() {
       sniffer.history = true;
       browser.url('http://new.org');
@@ -1047,6 +1055,32 @@ describe('browser', function() {
         expect($location.absUrl()).toEqual('http://server/#otherHash');
       });
     });
+
+    // issue #16632
+    it('should not trigger `$locationChangeStart` more than once due to trailing `#`', function() {
+      setup({
+        history: true,
+        html5Mode: true
+      });
+
+      inject(function($flushPendingTasks, $location, $rootScope) {
+        $rootScope.$digest();
+
+        const spy = jasmine.createSpy('$locationChangeStart');
+        $rootScope.$on('$locationChangeStart', spy);
+
+        $rootScope.$evalAsync(function() {
+          fakeWindow.location.href += '#';
+        });
+        $rootScope.$digest();
+
+        expect(fakeWindow.location.href).toBe('http://server/#');
+        expect($location.absUrl()).toBe('http://server/');
+
+        expect(spy.calls.count()).toBe(0);
+        expect(spy).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('integration test with $rootScope', function() {

test/ng/locationSpec.js

@@ -709,7 +709,7 @@ describe('$location', function() {
       initService({html5Mode: true, supportHistory: true});
       mockUpBrowser({initialUrl: 'http://new.com/#', baseHref: '/'});
       inject(function($browser, $location, $window) {
-        expect($browser.url()).toBe('http://new.com/#');
+        expect($browser.url()).toBe('http://new.com/');
         expect($location.absUrl()).toBe('http://new.com/');
         expect($window.location.href).toBe('http://new.com/#');
       });