Skip to content

Commit f774412

Browse files
gkalpakpetebacondarwin
authored andcommitted
fix($anchorScroll): use the load event instead of readystatechange
1 parent 28a9ffb commit f774412

File tree

2 files changed

+45
-44
lines changed

2 files changed

+45
-44
lines changed

src/ng/anchorScroll.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,13 @@ function $AnchorScrollProvider() {
237237
$rootScope.$evalAsync(scroll);
238238
} else if (!scrollScheduled) {
239239
scrollScheduled = true;
240-
document.addEventListener('readystatechange', function unbindAndScroll() {
240+
angular.element($window).on('load', function unbindAndScroll() {
241241
// When navigating to a page with a URL including a hash,
242242
// Firefox overwrites our `yOffset` if `$apply()` is used instead.
243243
$rootScope.$evalAsync(function() {
244-
if (document.readyState === 'complete') {
245-
scrollScheduled = false;
246-
document.removeEventListener('readystatechange', unbindAndScroll);
247-
scroll();
248-
}
244+
scrollScheduled = false;
245+
angular.element($window).off('load', unbindAndScroll);
246+
scroll();
249247
});
250248
});
251249
}

test/ng/anchorScrollSpec.js

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,26 @@ describe('$anchorScroll', function() {
9494
elmSpy = {};
9595
windowSpies = {};
9696

97-
$provide.value('$window', {
97+
var mockedWin = {
9898
scrollTo: (windowSpies.scrollTo = jasmine.createSpy('$window.scrollTo')),
9999
scrollBy: (windowSpies.scrollBy = jasmine.createSpy('$window.scrollBy')),
100100
document: createMockDocument(initialReadyState),
101101
navigator: {},
102102
getComputedStyle: function(elem) {
103103
return getComputedStyle(elem);
104+
},
105+
addEventListener: function(eventType, callback, unsupported) {
106+
window.addEventListener(eventType, callback, unsupported);
107+
},
108+
removeEventListener: function(eventType, callback, unsupported) {
109+
window.removeEventListener(eventType, callback, unsupported);
104110
}
105-
});
111+
};
112+
113+
windowSpies.addEventListener = spyOn(mockedWin, 'addEventListener').andCallThrough();
114+
windowSpies.removeEventListener = spyOn(mockedWin, 'removeEventListener').andCallThrough();
115+
116+
$provide.value('$window', mockedWin);
106117
});
107118
};
108119
}
@@ -152,16 +163,6 @@ describe('$anchorScroll', function() {
152163
};
153164
}
154165

155-
function updateMockReadyState(newState) {
156-
return function($browser, $window) {
157-
// It is possible that this operation adds tasks to the asyncQueue (needs flushing)
158-
$window.document.updateReadyState(newState);
159-
if ($browser.deferredFns.length) {
160-
$browser.defer.flush();
161-
}
162-
};
163-
}
164-
165166

166167
afterEach(inject(function($browser, $document) {
167168
expect($browser.deferredFns.length).toBe(0);
@@ -223,71 +224,73 @@ describe('$anchorScroll', function() {
223224

224225
describe('with respect to `document.readyState`', function() {
225226

227+
function triggerLoadEvent() {
228+
return function($browser, $window) {
229+
// It is possible that this operation adds tasks to the asyncQueue (needs flushing)
230+
$window.document.readyState = 'complete';
231+
jqLite($window).triggerHandler('load');
232+
if ($browser.deferredFns.length) {
233+
$browser.defer.flush();
234+
}
235+
};
236+
}
237+
226238
beforeEach(createMockWindow('interactive'));
227239

228240

229-
it('should wait for `document.readyState === "complete"', inject(
241+
it('should wait for the `load` event', inject(
230242
addElements('id=some1'),
231243

232244
changeHashTo('some1'),
233245
expectNoScrolling(),
234246

235-
updateMockReadyState('some-arbitrary-state'),
236-
expectNoScrolling(),
237-
238-
updateMockReadyState('complete'),
247+
triggerLoadEvent(),
239248
expectScrollingTo('id=some1')));
240249

241250

242-
it('should only register once for execution when `document.readyState === "complete"', inject(
251+
it('should only register one listener while `readyState !== "complete"`', inject(
243252
addElements('id=some1', 'id=some2'),
244253

245-
changeHashTo('some1'),
246-
changeHashTo('some2'),
247-
updateMockReadyState('some-other-arbitrary-state'),
248254
changeHashTo('some1'),
249255
changeHashTo('some2'),
250256
expectNoScrolling(),
251257

252-
updateMockReadyState('complete'),
258+
triggerLoadEvent(),
253259
expectScrollingTo('id=some2')));
254260

255261

256-
it('should properly register and unregister listeners for `readystatechange` event', inject(
262+
it('should properly register and unregister listeners for the `load` event', inject(
257263
addElements('id=some1', 'id=some2'),
258264

259265
changeHashTo('some1'),
260266
changeHashTo('some2'),
261-
updateMockReadyState('some-other-arbitrary-state'),
262-
changeHashTo('some1'),
263-
changeHashTo('some2'),
264-
updateMockReadyState('complete'),
267+
triggerLoadEvent(),
265268

266269
function() {
267-
expect(docSpies.addEventListener.callCount).toBe(1);
268-
expect(docSpies.addEventListener).
269-
toHaveBeenCalledWith('readystatechange', jasmine.any(Function));
270+
expect(windowSpies.addEventListener.callCount).toBe(1);
271+
expect(windowSpies.addEventListener).
272+
toHaveBeenCalledWith('load', jasmine.any(Function), false);
270273

271-
expect(docSpies.removeEventListener.callCount).toBe(1);
272-
expect(docSpies.removeEventListener).
273-
toHaveBeenCalledWith('readystatechange', jasmine.any(Function));
274+
expect(windowSpies.removeEventListener.callCount).toBe(1);
275+
expect(windowSpies.removeEventListener).
276+
toHaveBeenCalledWith('load', jasmine.any(Function), false);
274277

275-
var registeredListener = docSpies.addEventListener.calls[0].args[1];
276-
var unregisteredListener = docSpies.removeEventListener.calls[0].args[1];
278+
var registeredListener = windowSpies.addEventListener.calls[0].args[1];
279+
var unregisteredListener = windowSpies.removeEventListener.calls[0].args[1];
277280
expect(unregisteredListener).toBe(registeredListener);
278281
}));
279282

280283

281284
it('should scroll immediately if already `readyState === "complete"`', inject(
282285
addElements('id=some1'),
283286

284-
updateMockReadyState('complete'),
287+
triggerLoadEvent(),
285288
changeHashTo('some1'),
286289

287290
expectScrollingTo('id=some1'),
288291
function() {
289-
expect(docSpies.addEventListener.callCount).toBe(0);
290-
expect(docSpies.removeEventListener.callCount).toBe(0);
292+
expect(windowSpies.addEventListener.callCount).toBe(0);
293+
expect(windowSpies.removeEventListener.callCount).toBe(0);
291294
}));
292295
});
293296

0 commit comments

Comments
 (0)