From e4b395258904f24ffa1b82fa56c8cd9d3846e4f9 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Mon, 22 Jan 2018 13:31:11 -0500 Subject: [PATCH 1/4] Clone options before modifying it for form submissions --- lib/proto/attach-form.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/proto/attach-form.js b/lib/proto/attach-form.js index 96bf39d..1a0ca83 100644 --- a/lib/proto/attach-form.js +++ b/lib/proto/attach-form.js @@ -6,14 +6,18 @@ var clone = require("../clone") var attrClick = "data-pjax-click-state" var formAction = function(el, event) { - this.options.requestOptions = { + // Since we'll be modifying request options, clone the existing options + // so these changes don't persist + var options = clone(this.options); + + options.requestOptions = { requestUrl: el.getAttribute("action") || window.location.href, requestMethod: el.getAttribute("method") || "GET", } // create a testable virtual link of the form action var virtLinkElement = document.createElement("a"); - virtLinkElement.setAttribute("href", this.options.requestOptions.requestUrl); + virtLinkElement.setAttribute("href", options.requestOptions.requestUrl); // Ignore external links. if (virtLinkElement.protocol !== window.location.protocol || virtLinkElement.host !== window.location.host) { @@ -34,7 +38,7 @@ var formAction = function(el, event) { } // if declared as a full reload, just normally submit the form - if (this.options.currentUrlFullReload) { + if (options.currentUrlFullReload) { el.setAttribute(attrClick, "reload"); return; } @@ -56,12 +60,12 @@ var formAction = function(el, event) { // Creating a getString var paramsString = (paramObject.map(function(value) {return value.name + "=" + value.value;})).join("&"); - this.options.requestOptions.requestPayload = paramObject; - this.options.requestOptions.requestPayloadString = paramsString; + options.requestOptions.requestPayload = paramObject; + options.requestOptions.requestPayloadString = paramsString; el.setAttribute(attrClick, "submit"); - var options = clone(this.options); + options.triggerElement = el; this.loadUrl(virtLinkElement.href, options); }; -- 2.49.1 From 526a0883a20a6a3f3b188638840ffe1ea170bc9f Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 23 Jan 2018 13:22:31 -0500 Subject: [PATCH 2/4] Clone options in attach-link to prevent requestOptions changes from persisting --- lib/proto/attach-form.js | 5 +++-- lib/proto/attach-link.js | 10 ++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/proto/attach-form.js b/lib/proto/attach-form.js index 1a0ca83..f6775a2 100644 --- a/lib/proto/attach-form.js +++ b/lib/proto/attach-form.js @@ -6,10 +6,11 @@ var clone = require("../clone") var attrClick = "data-pjax-click-state" var formAction = function(el, event) { - // Since we'll be modifying request options, clone the existing options - // so these changes don't persist + // 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); + // Initialize requestOptions options.requestOptions = { requestUrl: el.getAttribute("action") || window.location.href, requestMethod: el.getAttribute("method") || "GET", diff --git a/lib/proto/attach-link.js b/lib/proto/attach-link.js index 2a0e641..69945bc 100644 --- a/lib/proto/attach-link.js +++ b/lib/proto/attach-link.js @@ -7,6 +7,13 @@ var attrClick = "data-pjax-click-state" var attrKey = "data-pjax-keyup-state" var linkAction = function(el, event) { + // 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); + + // Initialize requestOptions since loadUrl expects it to be an object + options.requestOptions = {}; + // 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") @@ -51,10 +58,9 @@ var linkAction = function(el, event) { this.reload() return } - this.options.requestOptions = this.options.requestOptions || {}; + el.setAttribute(attrClick, "load") - var options = clone(this.options) options.triggerElement = el this.loadUrl(el.href, options) } -- 2.49.1 From f196604d7391e90a3d7298c3bf1a59d56fc19133 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 23 Jan 2018 13:22:48 -0500 Subject: [PATCH 3/4] Add tests to ensure options are not accidentally modified --- package.json | 2 +- tests/lib/proto/attach-form.js | 18 ++++++++++++++++++ tests/lib/proto/attach-link.js | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 5d2949c..c23931e 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "lint": "jscs . && jshint . --exclude-path .gitignore", "standalone": "browserify index.js --standalone Pjax > pjax.js", "build-debug": "browserify index.js --debug --standalone Pjax > pjax.js", - "tests": "tape -r ./tests/setup.js ./tests/**/*.js", + "tests": "tape -r ./tests/setup.js ./tests/lib/*.js ./tests/lib/proto/*.js", "test": "npm run lint && npm run tests | tap-spec", "coverage-tests": "npm run tests | tap-nyc", "coverage": "nyc -x \"tests/**\" npm run coverage-tests", diff --git a/tests/lib/proto/attach-form.js b/tests/lib/proto/attach-form.js index 701b89f..ab89543 100644 --- a/tests/lib/proto/attach-form.js +++ b/tests/lib/proto/attach-form.js @@ -76,3 +76,21 @@ tape("test attach form preventDefaulted events", function(t) { t.end() }) + +tape("test options are not modified by attachForm", function(t) { + var form = document.createElement("form") + var options = {foo: "bar"} + var loadUrl = () => {} + + attachForm.call({options, loadUrl}, form) + + var internalUri = window.location.protocol + "//" + window.location.host + window.location.pathname + window.location.search + form.action = internalUri + form.method = "GET" + trigger(form, "submit") + + t.equal(1, Object.keys(options).length, "options object that is passed in should not be modified") + t.equal("bar", options.foo, "options object that is passed in should not be modified") + + t.end(); +}) diff --git a/tests/lib/proto/attach-link.js b/tests/lib/proto/attach-link.js index fef1bfc..fbaae4e 100644 --- a/tests/lib/proto/attach-link.js +++ b/tests/lib/proto/attach-link.js @@ -75,3 +75,21 @@ tape("test attach link preventDefaulted events", function(t) { t.end() }) + +tape("test options are not modified by attachLink", function(t) { + var a = document.createElement("a") + var options = {foo: "bar"} + var loadUrl = () => {}; + + attachLink.call({options, loadUrl}, a) + + var internalUri = window.location.protocol + "//" + window.location.host + window.location.pathname + window.location.search + a.href = internalUri + + trigger(a, "click") + + t.equal(1, Object.keys(options).length, "options object that is passed in should not be modified") + t.equal("bar", options.foo, "options object that is passed in should not be modified") + + t.end(); +}) -- 2.49.1 From feb85382f2a4cfb5d004326b734b884013d525d2 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 23 Jan 2018 15:05:31 -0500 Subject: [PATCH 4/4] Revert back to using '**' glob for tests, wrap in quotes to force node to parse the args --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c23931e..a317664 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "lint": "jscs . && jshint . --exclude-path .gitignore", "standalone": "browserify index.js --standalone Pjax > pjax.js", "build-debug": "browserify index.js --debug --standalone Pjax > pjax.js", - "tests": "tape -r ./tests/setup.js ./tests/lib/*.js ./tests/lib/proto/*.js", + "tests": "tape -r ./tests/setup.js './tests/**/*.js'", "test": "npm run lint && npm run tests | tap-spec", "coverage-tests": "npm run tests | tap-nyc", "coverage": "nyc -x \"tests/**\" npm run coverage-tests", -- 2.49.1