closure-library icon indicating copy to clipboard operation
closure-library copied to clipboard

Make Html5History ignore initial navigation event on page load in Safari

Open metametadata opened this issue 9 years ago • 1 comments

Actual Safari fires popstate event on initial page load. So the Html5History event callback is triggered twice leading to potential issues.

Expected It would be great if Closure lib ignored Safari's initial event to make Html5History work the same as in Chrome and FF: on page load navigation event should be triggered only once - on Html5History instance's setEnabled(true).

Environment Reproduced in Safari 9, using ClojureScript 1.8.34 which includes:

[com.google.javascript/closure-compiler "v20151216"] [org.clojure/google-closure-library "0.0-20151016-61277aea"] [org.clojure/google-closure-library-third-party "0.0-20151016-61277aea"]

Workarounds In a meantime, it looks like users have to implement workarounds, e.g.:

  1. https://github.com/circleci/frontend/blob/2064f2a6fb3b883d2a965913352cdd46bd197a40/src-cljs/frontend/history.cljs#L53

  2. use goog.History instead of goog.history.Html5History.

  3. http://anmonteiro.com/2015/09/solving-closure-librarys-html5history-double-event-dispatch/ (NOTE: unfortunately this approach breaks the Back button, proof: http://v.gd/apribase_safari_issue)

Example

<html>
<head>
    <script src="../closure-library/closure/goog/base.js"></script>
    <script>
        goog.require('goog.history.Html5History');
        goog.require('goog.debug.DivConsole');
        goog.require('goog.dom');
        goog.require('goog.events');
        goog.require('goog.history.EventType');
        goog.require('goog.log');
        goog.require('goog.object');
        goog.require('goog.string');
    </script>
</head>
<body>
<fieldset class="goog-debug-panel">
    <legend>Event Log</legend>
    <div id="log"></div>
</fieldset>

<script>
  var logger = goog.log.getLogger('demo');
  var logconsole = new goog.debug.DivConsole(goog.dom.getElement('log'));
  logconsole.setCapturing(true);

  var events = goog.object.getValues(goog.history.EventType);
  goog.log.info(logger, 'Listening for: ' + events.join(', ') + '.');

  var h = new goog.history.Html5History();
  goog.events.listen(h, events, function(e) {
    goog.log.info(logger, goog.string.subs('dispatched: %s (token="%s", isNavigation=%s)',
        e.type, e.token, e.isNavigation));
  });

  h.setEnabled(true);
</script>
</body>
</html>

Opening this file in Chrome 49 shows log:

[  0.012s] [demo] Listening for: navigate.
[  0.017s] [demo] dispatched: navigate (token="", isNavigation=false)

In Safari 9:

[  0.006s] [demo] Listening for: navigate.
[  0.008s] [demo] dispatched: navigate (token="", isNavigation=false)
[  0.009s] [demo] dispatched: navigate (token="", isNavigation=true)

metametadata avatar Mar 21 '16 22:03 metametadata

I'm not clear on what the solution should be. As I understand it, goog.Html5History should probably be preferred over goog.History, so workaround #2 is out. And as you point out, simply unlistening popstate (as the other two solutions seem to be doing, though admittedly I don't really grok ClojureScript) doesn't solve the problem because it breaks the Back button.

If you could provide more details (or even better, a patch), then we could look into it further. Perhaps a workaround would be to detect and swallow a duplicate event in onHistoryEvent_? But I'm not familiar enough with the circumstances to suggest how that might be detected. Is this bug also present in older versions of Safari, or only 9? (What about future versions? is it just an incompatibility with the spec? does the spec say anything about whether the browser should fire this popstate event or not?)

shicks avatar Apr 11 '16 21:04 shicks