diff --git a/lib/timeline-window.js b/lib/timeline-window.js index 84ad9c009..67e949c47 100644 --- a/lib/timeline-window.js +++ b/lib/timeline-window.js @@ -30,6 +30,13 @@ var DEBUG = false; */ var debuglog = DEBUG ? console.log.bind(console) : function() {}; +/** + * the number of times we ask the server for more events before giving up + * + * @private + */ +var DEFAULT_PAGINATE_LOOP_LIMIT = 5; + /** * Construct a TimelineWindow. * @@ -179,10 +186,14 @@ TimelineWindow.prototype.canPaginate = function(direction) { * even if there are fewer than 'size' of them, as we will just return those * we already know about.) * + * @param {number} [requestLimit=5] limit for the number of API requests we should + * make. + * * @return {module:client.Promise} Resolves to a boolean which is true if more events * were successfully retrieved. */ -TimelineWindow.prototype.paginate = function(direction, size, makeRequest) { +TimelineWindow.prototype.paginate = function(direction, size, makeRequest, + requestLimit) { // Either wind back the message cap (if there are enough events in the // timeline to do so), or fire off a pagination request. @@ -190,6 +201,10 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest) { makeRequest = true; } + if (requestLimit === undefined) { + requestLimit = DEFAULT_PAGINATE_LOOP_LIMIT; + } + var tl; if (direction == EventTimeline.BACKWARDS) { tl = this._start; @@ -224,7 +239,9 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest) { return q(true); } - if (!makeRequest) { + if (!makeRequest || requestLimit === 0) { + // todo: should we return something different to indicate that there + // might be more envets out there, but we haven't found them yet? return q(false); } @@ -256,7 +273,12 @@ TimelineWindow.prototype.paginate = function(direction, size, makeRequest) { // token. In particular, we want to know if we've actually hit the // start of the timeline, or if we just happened to know about all of // the events thanks to https://matrix.org/jira/browse/SYN-645. - return self.paginate(direction, size, true); + // + // On the other hand, we necessarily want to wait forever for the + // server to make its mind up about whether there are other events, + // because it gives a bad user experience + // (https://github.com/vector-im/vector-web/issues/1204). + return self.paginate(direction, size, true, requestLimit - 1); }); tl.pendingPaginate = prom; return prom; diff --git a/spec/unit/timeline-window.spec.js b/spec/unit/timeline-window.spec.js index 177dc5dce..ee85b5fe5 100644 --- a/spec/unit/timeline-window.spec.js +++ b/spec/unit/timeline-window.spec.js @@ -418,5 +418,43 @@ describe("TimelineWindow", function() { }).catch(utils.failTest).done(done); }); + it("should limit the number of unsuccessful pagination requests", + function(done) { + var timeline = createTimeline(); + timeline.setPaginationToken("toktok", EventTimeline.FORWARDS); + + var timelineWindow = createWindow(timeline, {windowLimit: 5}); + var eventId = timeline.getEvents()[1].getId(); + + var paginateCount = 0; + client.paginateEventTimeline = function(timeline0, opts) { + expect(timeline0).toBe(timeline); + expect(opts.backwards).toBe(false); + expect(opts.limit).toEqual(2); + paginateCount += 1 + return q(true); + }; + + timelineWindow.load(eventId, 3).then(function() { + var expectedEvents = timeline.getEvents(); + expect(timelineWindow.getEvents()).toEqual(expectedEvents); + + expect(timelineWindow.canPaginate(EventTimeline.BACKWARDS)) + .toBe(false); + expect(timelineWindow.canPaginate(EventTimeline.FORWARDS)) + .toBe(true); + return timelineWindow.paginate(EventTimeline.FORWARDS, 2, true, 3); + }).then(function(success) { + expect(success).toBe(false); + expect(paginateCount).toEqual(3); + var expectedEvents = timeline.getEvents().slice(0, 3); + expect(timelineWindow.getEvents()).toEqual(expectedEvents); + + expect(timelineWindow.canPaginate(EventTimeline.BACKWARDS)) + .toBe(false); + expect(timelineWindow.canPaginate(EventTimeline.FORWARDS)) + .toBe(true); + }).catch(utils.failTest).done(done); + }); }); });