olivernn / davis.js

RESTful degradable JavaScript routing using pushState
http://davisjs.com
532 stars 58 forks source link

If form URL contains a query string, form contents are dropped #75

Open dumbmatter opened 11 years ago

dumbmatter commented 11 years ago

Example: http://jsfiddle.net/TBGaX/

These two forms:

<form action="/test_case?k1=whatever" method="get">
  <input type="hidden" name="k2" value="blah">
  <button type="submit">Query string in form URL</button>
</form>
<form action="/test_case" method="get">
  <input type="hidden" name="k2" value="blah">
  <button type="submit">No query string in form URL</button>
</form>

<p id="log">Requests recieved:</p>

backed by this JavaScript:

var app = new Davis(function () {
    this.get("/test_case", function (req) {
        document.getElementById("log").innerHTML += "<br>k2: " + req.params.k2;
        console.log(req);
    });
});

app.start();

should produce the same output, since the k1 parameter doesn't do anything. But submitting the first form results in req.params.k2 being undefined. This is because req.fullPath is set to "/test_case?k1=whatever?k2=blah" (which is obviously not right).

I did a little debugging and found that the source of the problem is this line in davis.listener.js:

      fullPath: (this.serialize() ? [this.prop('action'), this.serialize()].join("?") : this.prop('action')),

Specifically this part [this.prop('action'), this.serialize()].join("?") is just joining the form URL with the form data by appending a new query string to the URL regardless of if one is already there.

I could imagine a quick fix, but I figured I'd submit a bug report rather than a pull request because I'm not sure if there are nuances involved in detecting when a query string is present or if it is actually appropriate to simply merge the query string with the form data.