#458 ✓invalid
Przemek

Form#serialize fails for forms interleaved inside tables

Reported by Przemek | November 21st, 2008 @ 04:29 AM

Here's a simple example:

<table>
  <form>
    <tr><td><input name="something" /></td></tr>
  </form>
</table>

In most browsers (excluding IE) this creates the following DOM hierarchy:

+table
  +form
  +tr
    +td
      +input

The browser knows what to submit because the input element is linked to the form via the form#elements collection.

One way to fix this so that Form#serialize and Form#request work in this case as well would be to redefine Form.Methods.getElements as follows:

Form.Methods.getElements = function(form) {
  return $A(form.elements).inject([],
    function(elements, child) {
      if (Form.Element.Serializers[child.tagName.toLowerCase()])
        elements.push(Element.extend(child));
      return elements;
    }
  );
}

This fixes my issue although I'm not sure if it will break anything for anyone else so I'm interested in some discussion.

Comments and changes to this ticket

  • Tobie Langel

    Tobie Langel November 22nd, 2008 @ 05:12 PM

    • State changed from “new” to “bug”
    • Tag set to needs_failing_test_case

    Could you kindly explain precisely what your issue is, ideally providing a failing test case.

    Thank you.

  • Przemek

    Przemek November 23rd, 2008 @ 04:40 AM

    Ok, I've attached a failing test case. The box at the bottom should read: Serialised form: input1=&input2=&submit=Submit

    Instead it reads: Serialised form:

    My initial thought was to fix this like so:

    --- a/src/form.js
    +++ b/src/form.js
    @@ -35,7 +35,7 @@ Form.Methods = {
       },
    
       getElements: function(form) {
    -    return $A($(form).getElementsByTagName('*')).inject([],
    +    return $A(form.elements).inject([],
           function(elements, child) {
             if (Form.Element.Serializers[child.tagName.toLowerCase()])
               elements.push(Element.extend(child));
    
    

    However this breaks quite a few test cases so obviously requires a bit more thought.

    A few things to mention. First this won't fail in IE because of how it handles interleaved forms and tables. It will fail in every other browser I know of though.

    Second, FireFox if presented with the failing HTML and told to parse it in JavaScript (say using innerHTML) will actually pass the test (even though now it has totaly broken its own ability to render the HTML correctly... that's another bug I have to chase up). This may be relevant to anyone writing a test case.

    By the way, is it possible to remove my first comment? It was kind of unncessary.

    Cheers

  • Juriy Zaytsev

    Juriy Zaytsev November 23rd, 2008 @ 05:46 AM

    • State changed from “bug” to “invalid”

    (Deleted first comment as you asked)

    The markup in your example is broken - you can't inject FORM in between TABLE and TR ones - that's not a valid markup (as per HTML 4.01 grammar). There's also unclosed span tag, no doctype, no required title attribute, etc.

    When tested within a proper document, it works as expected:

    
    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
      "http://www.w3.org/TR/html4/strict.dtd">
    
    <html>
      <head>
        <meta http-equiv="Content-type" content="text/html; charset=utf-8">
        <title></title>
        <script src="http://ajax.googleapis.com/ajax/libs/prototype/1.6.0.3/prototype.js" type="text/javascript"></script>
      </head>
      <body>
        <form id="form" action="">
          <table>
            <tr><td>Input 1</td><td><input name="input1" type="text"></td></tr>
            <tr><td>Input 2</td><td><input name="input2" type="text"></td></tr>
            <tr><td colspan="2"><input name="submit" type="submit"></td></tr>
          </table>
        </form>
        <div id="result" style="border: 1px solid black;">
          Serialised form:
          <span id="serialised"></span>
        </div>
        <script type="text/javascript">
          $('serialised').innerHTML = $('form').serialize(false);
        </script>
      </body>
    </html>
    

    Closing as invalid. Feel free to reopen if I missed something.

  • Przemek

    Przemek November 24th, 2008 @ 01:04 AM

    I was using XHTML for my markup without qualification, sorry for the confusion this created, I wasn't aiming at pedantism ;)

    I'm aware forms nested in tables are not valid markup as per HTML 4.01, they are however supported by every browser and at the current time are the only way to align multiple forms together. As a result the Prototype handling of forms seems to be incompatible with most browsers. So we're talking about an architectural problems that may rear its head in other weird cases.

    Having said that, given this behaviour is supported de facto by all widespread implementations of HTML 4.01 and not the standard itself it may be reasonable to ignore it... probably. I'd re-open the bug to finalise discussion but presumably don't have the priveleges, here's a failing test-case that better demonstrates my intention with Juriy's changes:

    
    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
      "http://www.w3.org/TR/html4/strict.dtd">
    
    <html>
      <head>
        <meta http-equiv="Content-type" content="text/html; charset=utf-8">
        <title></title>
        <script src="http://ajax.googleapis.com/ajax/libs/prototype/1.6.0.3/prototype.js" type="text/javascript"></script>
      </head>
      <body>
        <table>
          <form id="form" action="">
            <tr><td>Input A1</td><td><input name="input1" type="text"></td></tr>
            <tr><td>Input A2</td><td><input name="input2" type="text"></td></tr>
            <tr><td colspan="2"><input name="submit" type="submit"></td></tr>
          </form>
          <form id="another-form" action="">
            <tr><td>Input B1</td><td><input name="input1b" type="text"></td></tr>
            <tr><td>Input B2</td><td><input name="input2b" type="text"></td></tr>
            <tr><td colspan="2"><input name="submit" type="submit"></td></tr>
          </form>
        </table>
        <div id="result" style="border: 1px solid black;">
          Serialised form:
          <span id="serialised"></span>
        </div>
        <script type="text/javascript">
          $('serialised').innerHTML = $('form').serialize(false);
        </script>
      </body>
    </html>
    
  • Juriy Zaytsev

    Juriy Zaytsev November 24th, 2008 @ 04:12 AM

    I see what you mean.

    It's just that keeping framework working in "broken" environment hasn't been our priority so far : ) Quirks mode, for example, currently breaks some of the position/dimension-related methods.

    Most of the time it's not worth it, though this particular case seems to be quite easy to "fix".

    On the other hand, wouldn't using a separate table element for each form work for you?

  • Przemek

    Przemek November 24th, 2008 @ 04:51 AM

    I only wish it would :( However I need all the columns to align across forms. The only other option is to set fixed width for the column elements. There will be ways to do this with CSS3 but that won't do me any good for quite a while yet.

    In my own application I've gotten around this issue by using Form.serializeElements($A(form.elements), true). Given how many unit tests my proposed fix seems to break I'm not sure if this is actually worth the effort although I probably lack much Prototype insight.

    For now maybe it makes sense to leave this open with a low priority (or change status to won't fix for posterity's sake).

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:40 AM

    • Tag changed from needs_failing_test_case to needs:failing_testcase

    [not-tagged:"needs_failing_test_case" tagged:"needs:failing_testcase" bulk edit command]

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

People watching this ticket

Attachments

Pages