Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the previous history rewriting logic #6200

Merged
merged 1 commit into from Sep 27, 2015

Commits on Sep 26, 2015

  1. 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.
    Rob--W committed Sep 26, 2015
    Configuration menu
    Copy the full SHA
    cdea75d View commit details
    Browse the repository at this point in the history