[Pkg-javascript-commits] [pdf.js] 20/174: Refactor the previous history rewriting logic
David Prévot
taffit at moszumanska.debian.org
Thu Nov 19 18:45:00 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository pdf.js.
commit cdea75dc397f4eb4d6fd1f7d8a388c7d11df3452
Author: Rob Wu <rob at robwu.nl>
Date: Tue Jul 14 23:33:37 2015 +0200
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.
---
web/pdf_history.js | 88 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 32 deletions(-)
diff --git a/web/pdf_history.js b/web/pdf_history.js
index a2fe22e..99f2553 100644
--- a/web/pdf_history.js
+++ b/web/pdf_history.js
@@ -75,38 +75,74 @@ 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.
- // If the history is empty when the hash changes,
- // update the previous entry in the browser history.
- if (self.uid === 0) {
- var previousParams = (self.previousHash && self.currentBookmark &&
+ // 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 (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;
- }
- self._pushToHistory({hash: self.previousHash}, false, true);
- self._updatePreviousBookmark();
+ 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();
+ }
+
+ 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);
if (previousParams) {
@@ -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() {
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/pdf.js.git
More information about the Pkg-javascript-commits
mailing list