Fix bugs and add tests (#145)

* Fix bug when checking if elements were parsed already

parse-element.js checks if the element was already parsed by
checking for the `data-pjax-click-state` attribute. However, this
attribute was not added until the link is clicked.

Originally, there was a separate attribute, `data-pjax-enabled`,
which tracked if the element was parsed already, but that was
changed in 9a86044.

This commit merges the attributes for mouse clicks and key presses
into one and adds that attribute when the element is initially
parsed.

* More bug fixes

* Fix documentation for currentUrlFullReload

* Ignore lines from coverage if they can't be tested

* Refactor attach-link and attach-form

* Fix and refactors tests

* Add tests

* Add TS definitions for options.requestOptions

* Code cleanup

lib/eval-script.js

@@ -13,6 +13,7 @@ module.exports = function(el) {
 
   script.type = "text/javascript"
 
+  /* istanbul ignore if */
   if (src !== "") {
     script.src = src
     script.async = false // force synchronous loading of peripheral JS
@@ -23,6 +24,7 @@ module.exports = function(el) {
       script.appendChild(document.createTextNode(code))
     }
     catch (e) {
+      /* istanbul ignore next */
       // old IEs have funky script nodes
       script.text = code
     }
@@ -31,7 +33,7 @@ module.exports = function(el) {
   // execute
   parent.appendChild(script)
   // avoid pollution only in head or body tags
-  if (["head", "body"].indexOf(parent.tagName.toLowerCase()) > 0) {
+  if (parent instanceof HTMLHeadElement || parent instanceof HTMLBodyElement) {
     parent.removeChild(script)
   }
 

lib/parse-options.js

@@ -9,16 +9,7 @@ module.exports = function(options) {
   options.switches = options.switches || {}
   options.switchesOptions = options.switchesOptions || {}
   options.history = options.history || true
-  options.analytics = (typeof options.analytics === "function" || options.analytics === false) ?
-    options.analytics :
-    function() {
-      if (window._gaq) {
-        _gaq.push(["_trackPageview"])
-      }
-      if (window.ga) {
-        ga("send", "pageview", {page: location.pathname, title: document.title})
-      }
-    }
+  options.analytics = (typeof options.analytics === "function" || options.analytics === false) ? options.analytics : defaultAnalytics
   options.scrollTo = (typeof options.scrollTo === "undefined") ? 0 : options.scrollTo
   options.scrollRestoration = (typeof options.scrollRestoration !== "undefined") ? options.scrollRestoration : true
   options.cacheBust = (typeof options.cacheBust === "undefined") ? true : options.cacheBust
@@ -38,3 +29,13 @@ module.exports = function(options) {
 
   return options
 }
+
+/* istanbul ignore next */
+function defaultAnalytics() {
+  if (window._gaq) {
+    _gaq.push(["_trackPageview"])
+  }
+  if (window.ga) {
+    ga("send", "pageview", {page: location.pathname, title: document.title})
+  }
+}

lib/proto/attach-form.js

@@ -1,9 +1,13 @@
 var on = require("../events/on")
 var clone = require("../util/clone")
 
-var attrClick = "data-pjax-click-state"
+var attrState = "data-pjax-state"
 
 var formAction = function(el, event) {
+  if (isDefaultPrevented(event)) {
+    return
+  }
+
   // Since loadUrl modifies options and we may add our own modifications below,
   // clone it so the changes don't persist
   var options = clone(this.options)
@@ -19,27 +23,9 @@ var formAction = function(el, event) {
   var virtLinkElement = document.createElement("a")
   virtLinkElement.setAttribute("href", options.requestOptions.requestUrl)
 
-  // Ignore external links.
-  if (virtLinkElement.protocol !== window.location.protocol || virtLinkElement.host !== window.location.host) {
-    el.setAttribute(attrClick, "external")
-    return
-  }
-
-  // Ignore click if we are on an anchor on the same page
-  if (virtLinkElement.pathname === window.location.pathname && virtLinkElement.hash.length > 0) {
-    el.setAttribute(attrClick, "anchor-present")
-    return
-  }
-
-  // Ignore empty anchor "foo.html#"
-  if (virtLinkElement.href === window.location.href.split("#")[0] + "#") {
-    el.setAttribute(attrClick, "anchor-empty")
-    return
-  }
-
-  // if declared as a full reload, just normally submit the form
-  if (options.currentUrlFullReload) {
-    el.setAttribute(attrClick, "reload")
+  var attrValue = checkIfShouldAbort(virtLinkElement, options)
+  if (attrValue) {
+    el.setAttribute(attrState, attrValue)
     return
   }
 
@@ -59,12 +45,34 @@ var formAction = function(el, event) {
     }
   }
 
-  el.setAttribute(attrClick, "submit")
+  el.setAttribute(attrState, "submit")
 
   options.triggerElement = el
   this.loadUrl(virtLinkElement.href, options)
 }
 
+function checkIfShouldAbort(virtLinkElement, options) {
+  // Ignore external links.
+  if (virtLinkElement.protocol !== window.location.protocol || virtLinkElement.host !== window.location.host) {
+    return "external"
+  }
+
+  // Ignore click if we are on an anchor on the same page
+  if (virtLinkElement.hash && virtLinkElement.href.replace(virtLinkElement.hash, "") === window.location.href.replace(location.hash, "")) {
+    return "anchor"
+  }
+
+  // Ignore empty anchor "foo.html#"
+  if (virtLinkElement.href === window.location.href.split("#")[0] + "#") {
+    return "anchor-empty"
+  }
+
+  // if declared as a full reload, just normally submit the form
+  if (options.currentUrlFullReload && virtLinkElement.href === window.location.href.split("#")[0]) {
+    return "reload"
+  }
+}
+
 var isDefaultPrevented = function(event) {
   return event.defaultPrevented || event.returnValue === false
 }
@@ -72,19 +80,13 @@ var isDefaultPrevented = function(event) {
 module.exports = function(el) {
   var that = this
 
-  on(el, "submit", function(event) {
-    if (isDefaultPrevented(event)) {
-      return
-    }
+  el.setAttribute(attrState, "")
 
+  on(el, "submit", function(event) {
     formAction.call(that, el, event)
   })
 
   on(el, "keyup", function(event) {
-    if (isDefaultPrevented(event)) {
-      return
-    }
-
     if (event.keyCode === 13) {
       formAction.call(that, el, event)
     }

lib/proto/attach-link.js

@@ -1,44 +1,20 @@
 var on = require("../events/on")
 var clone = require("../util/clone")
 
-var attrClick = "data-pjax-click-state"
-var attrKey = "data-pjax-keyup-state"
+var attrState = "data-pjax-state"
 
 var linkAction = function(el, event) {
+  if (isDefaultPrevented(event)) {
+    return
+  }
+
   // Since loadUrl modifies options and we may add our own modifications below,
   // clone it so the changes don't persist
   var options = clone(this.options)
 
-  // Don’t break browser special behavior on links (like page in new window)
-  if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) {
-    el.setAttribute(attrClick, "modifier")
-    return
-  }
-
-  // we do test on href now to prevent unexpected behavior if for some reason
-  // user have href that can be dynamically updated
-
-  // Ignore external links.
-  if (el.protocol !== window.location.protocol || el.host !== window.location.host) {
-    el.setAttribute(attrClick, "external")
-    return
-  }
-
-  // Ignore click if we are on an anchor on the same page
-  if (el.pathname === window.location.pathname && el.hash.length > 0) {
-    el.setAttribute(attrClick, "anchor-present")
-    return
-  }
-
-  // Ignore anchors on the same page (keep native behavior)
-  if (el.hash && el.href.replace(el.hash, "") === window.location.href.replace(location.hash, "")) {
-    el.setAttribute(attrClick, "anchor")
-    return
-  }
-
-  // Ignore empty anchor "foo.html#"
-  if (el.href === window.location.href.split("#")[0] + "#") {
-    el.setAttribute(attrClick, "anchor-empty")
+  var attrValue = checkIfShouldAbort(el, event)
+  if (attrValue) {
+    el.setAttribute(attrState, attrValue)
     return
   }
 
@@ -49,17 +25,42 @@ var linkAction = function(el, event) {
     this.options.currentUrlFullReload &&
     el.href === window.location.href.split("#")[0]
   ) {
-    el.setAttribute(attrClick, "reload")
+    el.setAttribute(attrState, "reload")
     this.reload()
     return
   }
 
-  el.setAttribute(attrClick, "load")
+  el.setAttribute(attrState, "load")
 
   options.triggerElement = el
   this.loadUrl(el.href, options)
 }
 
+function checkIfShouldAbort(el, event) {
+  // Don’t break browser special behavior on links (like page in new window)
+  if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) {
+    return "modifier"
+  }
+
+  // we do test on href now to prevent unexpected behavior if for some reason
+  // user have href that can be dynamically updated
+
+  // Ignore external links.
+  if (el.protocol !== window.location.protocol || el.host !== window.location.host) {
+    return "external"
+  }
+
+  // Ignore anchors on the same page (keep native behavior)
+  if (el.hash && el.href.replace(el.hash, "") === window.location.href.replace(location.hash, "")) {
+    return "anchor"
+  }
+
+  // Ignore empty anchor "foo.html#"
+  if (el.href === window.location.href.split("#")[0] + "#") {
+    return "anchor-empty"
+  }
+}
+
 var isDefaultPrevented = function(event) {
   return event.defaultPrevented || event.returnValue === false
 }
@@ -67,25 +68,13 @@ var isDefaultPrevented = function(event) {
 module.exports = function(el) {
   var that = this
 
-  on(el, "click", function(event) {
-    if (isDefaultPrevented(event)) {
-      return
-    }
+  el.setAttribute(attrState, "")
 
+  on(el, "click", function(event) {
     linkAction.call(that, el, event)
   })
 
   on(el, "keyup", function(event) {
-    if (isDefaultPrevented(event)) {
-      return
-    }
-
-    // Don’t break browser special behavior on links (like page in new window)
-    if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) {
-      el.setAttribute(attrKey, "modifier")
-      return
-    }
-
     if (event.keyCode === 13) {
       linkAction.call(that, el, event)
     }

lib/proto/parse-element.js

@@ -1,15 +1,17 @@
+var attrState = "data-pjax-state"
+
 module.exports = function(el) {
   switch (el.tagName.toLowerCase()) {
     case "a":
       // only attach link if el does not already have link attached
-      if (!el.hasAttribute("data-pjax-click-state")) {
+      if (!el.hasAttribute(attrState)) {
         this.attachLink(el)
       }
       break
 
     case "form":
       // only attach link if el does not already have link attached
-      if (!el.hasAttribute("data-pjax-click-state")) {
+      if (!el.hasAttribute(attrState)) {
         this.attachForm(el)
       }
       break

lib/send-request.js

@@ -15,7 +15,7 @@ module.exports = function(location, options, callback) {
       if (request.status === 200) {
         callback(request.responseText, request, location)
       }
-      else {
+      else if (request.status !== 0) {
         callback(null, request, location)
       }
     }
@@ -65,7 +65,7 @@ module.exports = function(location, options, callback) {
 
   // Send the proper header information for POST forms
   if (requestPayload && requestMethod === "POST") {
-    request.setRequestHeader("Content-type", "application/x-www-form-urlencoded")
+    request.setRequestHeader("Content-Type", "application/x-www-form-urlencoded")
   }
 
   request.send(requestPayload)

lib/switches.js

@@ -8,7 +8,14 @@ module.exports = {
 
   innerHTML: function(oldEl, newEl) {
     oldEl.innerHTML = newEl.innerHTML
-    oldEl.className = newEl.className
+
+    if (newEl.className === "") {
+      oldEl.removeAttribute("class")
+    }
+    else {
+      oldEl.className = newEl.className
+    }
+
     this.onSwitch()
   },
 

lib/util/clone.js

@@ -1,4 +1,5 @@
 module.exports = function(obj) {
+  /* istanbul ignore if */
   if (null === obj || "object" !== typeof obj) {
     return obj
   }

lib/util/extend.js

@@ -1,6 +1,6 @@
 module.exports = function(target) {
   if (target == null) {
-    return target
+    return null
   }
 
   var to = Object(target)