Refactor the previous history rewriting logic

When the user edits the URL and changes the reference fragment (hash),
PDF.js intercepts this action, and saves the then-current history state
in the previous history entry. This is implemented by navigating back,
editing the history and navigating forward again.

The current logic has a flaw: It assumes that calling history.back() and
history.forward() immediately updates the history state. This is however
not guaranteed by the web standards, which states that calling e.g.
history.back "must traverse the history by a delta -1", which means that
the browser must QUEUE a task to traverse the session history, per spec:
http://w3.org/TR/2011/WD-html5-20110113/history.html#dom-history-back
https://html.spec.whatwg.org/multipage/browsers.html#dom-history-back

Firefox and Internet Explorer deviate from the standards by immediately
changing the history state instead of queuing the navigation.
WebKit derived browsers (Chrome, Opera, Safari) and Opera presto do not.

The user-visible consequence of strictly adhering to the standards in
PDF.js can be shown as follows:

1. Edit the URL.
2. Append #page=2 for example.
3. Press Enter.
   -> Presto and WebKit: PDF.js reverts to the previous URL.
   -> Gecko and Trident: PDF.js keeps the new URL, as expected.

To fix the issue, modification of the previous history item happens in
a few asynchronous steps, guided by the popstate event to detect when
the history navigation request has been committed.

--
Some more implementation notes:

I have removed the preventDefault and stopPropagation calls, because
popstate is not cancelable, and window is already the last target of the
event propagation.

The previous allowHashChange logic was hard to follow, because it did
not explain that hashchange will be called twice; once during the
popstate handler for history.back() (which will reset allowHashChange),
and again for history.forward() (where allowHashChange will be false).
The purpose of allowHashChange is now more explicit, by incorporating
the logic in the replacePreviousHistoryState helper function.
This commit is contained in:
Rob Wu 2015-07-14 23:33:37 +02:00
parent 7d4303b7c4
commit cdea75dc39

View File

@ -75,37 +75,73 @@ var PDFHistory = (function () {
var self = this;
window.addEventListener('popstate', function pdfHistoryPopstate(evt) {
evt.preventDefault();
evt.stopPropagation();
if (!self.historyUnlocked) {
return;
}
if (evt.state) {
// Move back/forward in the history.
self._goTo(evt.state);
} else {
// Handle the user modifying the hash of a loaded document.
self.previousHash = window.location.hash.substring(1);
return;
}
// If the state is not set, then the user tried to navigate to a
// different hash by manually editing the URL and pressing Enter, or by
// clicking on an in-page link (e.g. the "current view" link).
// Save the current view state to the browser history.
// Note: In Firefox, history.null could also be null after an in-page
// navigation to the same URL, and without dispatching the popstate
// event: https://bugzilla.mozilla.org/show_bug.cgi?id=1183881
// If the history is empty when the hash changes,
// update the previous entry in the browser history.
if (self.uid === 0) {
// Replace the previous state if it was not explicitly set.
var previousParams = (self.previousHash && self.currentBookmark &&
self.previousHash !== self.currentBookmark) ?
{hash: self.currentBookmark, page: self.currentPage} :
{page: 1};
self.historyUnlocked = false;
self.allowHashChange = false;
window.history.back();
self._pushToHistory(previousParams, false, true);
window.history.forward();
self.historyUnlocked = true;
replacePreviousHistoryState(previousParams, function() {
updateHistoryWithCurrentHash();
});
} else {
updateHistoryWithCurrentHash();
}
}, false);
function updateHistoryWithCurrentHash() {
self.previousHash = window.location.hash.slice(1);
self._pushToHistory({hash: self.previousHash}, false, true);
self._updatePreviousBookmark();
}
}, false);
function replacePreviousHistoryState(params, callback) {
// To modify the previous history entry, the following happens:
// 1. history.back()
// 2. _pushToHistory, which calls history.replaceState( ... )
// 3. history.forward()
// Because a navigation via the history API does not immediately update
// the history state, the popstate event is used for synchronization.
self.historyUnlocked = false;
// Suppress the hashchange event to avoid side effects caused by
// navigating back and forward.
self.allowHashChange = false;
window.addEventListener('popstate', rewriteHistoryAfterBack);
history.back();
function rewriteHistoryAfterBack() {
window.removeEventListener('popstate', rewriteHistoryAfterBack);
window.addEventListener('popstate', rewriteHistoryAfterForward);
self._pushToHistory(params, false, true);
history.forward();
}
function rewriteHistoryAfterForward() {
window.removeEventListener('popstate', rewriteHistoryAfterForward);
self.allowHashChange = true;
self.historyUnlocked = true;
callback();
}
}
function pdfHistoryBeforeUnload() {
var previousParams = self._getPreviousParams(null, true);
@ -178,19 +214,7 @@ var PDFHistory = (function () {
if (!this.initialized) {
return true;
}
// If the current hash changes when moving back/forward in the history,
// this will trigger a 'popstate' event *as well* as a 'hashchange' event.
// Since the hash generally won't correspond to the exact the position
// stored in the history's state object, triggering the 'hashchange' event
// can thus corrupt the browser history.
//
// When the hash changes during a 'popstate' event, we *only* prevent the
// first 'hashchange' event and immediately reset allowHashChange.
// If it is not reset, the user would not be able to change the hash.
var temp = this.allowHashChange;
this.allowHashChange = true;
return temp;
return this.allowHashChange;
},
_updatePreviousBookmark: function pdfHistory_updatePreviousBookmark() {