search

youtube / spfjs

A lightweight JS framework for fast navigation and page updates from YouTube
https://youtube.github.io/spfjs/
MIT License
2.23k stars 147 forks source link

Fix reloads when the URL contains a hash #381

Closed nicksay closed 8 years ago

nicksay commented 8 years ago

The previous change explicitly called window.location.reload() after assigning to window.location.href to ensure a reload happens for URLs that contain a hash. However, an immediate call to window.location.reload() will cancel navigation started by the assignment to window.location.href for other cases. Fix this by limiting calls to window.location.reload() for only those cases where it is needed.

Fixes #377

DavidCPhillips commented 8 years ago

LGTM. Is there anyway to cover this behavior in jasmine tests?

rviscomi commented 8 years ago

lgtm

nicksay commented 8 years ago

A test is a good idea. We have a couple options here:

  1. Update isNavigable_ to allow navigation from /page#target to /page, making these transitions eligible to send a request. For reference, Chrome makes a request in this case when clicking or going forward, but not when going back.
  2. Add a variable to adjust the behavior of isNavigable_ on demand.
  3. Add a new function with tests.
DavidCPhillips commented 8 years ago

I like the sound of Option 1. It's the most predictable behavior and in the majority of cases would just pull from cache anyway, so the performance hit should be minimal.

nicksay commented 8 years ago

PTAL I've implemented option 1 and updated the tests

DavidCPhillips commented 8 years ago

LGTM