Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

accept header with */* does not match html templates

#6379

See the comments attached to this commit for details:

https://github.com/rails/jquery-ujs/commit/22877193b17f2c257a83faca...

Reported by Alex Tambellini · February 5th, 2011 @ 10:27 PM

State: verified
Milestone: 3.1
Assigned to: Santiago Pastorino Santiago Pastorino
Importance: Low

Activity

  1. Santiago Pastorino
    Santiago Pastorino
    • State changed from new to open
    • Milestone set to 3.1
    • Importance changed from to Low

    Can you provide a test case?

    February 6th, 2011 @ 09:14 PM

  2. Alex Tambellini
    Alex Tambellini

    Attached is a rails app that demonstrates the bug:

    It has a html controller with an index action and a post action.

    The index action renders 2 forms that use :remote => true.

    Both forms submit to the post action with the accept header containing /. However, only the form that includes :"data-type" => "html" succeeds.

    Just start the app (rails 3.0.3) and go to localhost:3000 and then click on "Submit With Data Type" and "Submit Without Data Type" buttons to see the behavior described above.

    February 6th, 2011 @ 10:10 PM

  3. Mathieu Ravaux
    Mathieu Ravaux

    Here is a patch that solves this issue.
    I've tested it against Alex's app.

    February 27th, 2011 @ 01:34 AM

  4. Santiago Pastorino
    Santiago Pastorino
    • Assigned user set to Santiago Pastorino

    hey Methieu we need a test case to apply the patch.
    Thanks.

    February 27th, 2011 @ 04:01 AM

  5. Mathieu Ravaux
    Mathieu Ravaux

    Hi Santiago, thank you for your quick reply. It's encouraging for a first contribution !

    Here is the testcase:

    February 27th, 2011 @ 05:05 AM

  6. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    Hey Mathieu,

    Nice work on your first patch! The testcase looks good too.

    One thing: could you merge the testcase and the fix into one commit and supply a new patch? :)

    February 27th, 2011 @ 05:25 PM

  7. Mathieu Ravaux
  8. Jeff Kreeftmeijer
    Jeff Kreeftmeijer
    • State changed from open to verified

    Verified. The patch applies to master and the provided test fails without it. :)

    February 28th, 2011 @ 11:43 PM

  9. Jason Roth
  10. Jason Roth
    Jason Roth

    Ignore my comment above - I posted it to the wrong issue. Sorry -j

    May 7th, 2011 @ 09:35 PM

  11. Jason Noble
  12. Jason Noble
    Jason Noble

    This has been pushed to master and can be closed.

    May 7th, 2011 @ 11:36 PM