Tapestry
  1. Tapestry
  2. TAPESTRY-1745

Palette javascript errors in Internet Explorer

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.5
    • Fix Version/s: 5.0.7
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      A very simple <t:palette/> doesn't load properly in Internet Explorer (6.0 and 7.0). Javascript errors abound.

      1. 1745_patch.txt
        0.9 kB
        Robert Zeigler

        Issue Links

          Activity

          Hide
          Robert Zeigler added a comment -

          I can confirm that the palette is borked in at least IE 6, although the confirmation comes by guiding someone else through navigating to a page containing a palette and verifying that it didn't work... so I don't have exact exception messages at the moment. I'll dig into this one the next time I have window's handy.

          Show
          Robert Zeigler added a comment - I can confirm that the palette is borked in at least IE 6, although the confirmation comes by guiding someone else through navigating to a page containing a palette and verifying that it didn't work... so I don't have exact exception messages at the moment. I'll dig into this one the next time I have window's handy.
          Hide
          Robert Zeigler added a comment -

          After some tracking (caveat: only have access to IE 7 at the moment, so there could be further issues on IE 6...), I found two places where IE is choking.
          1) in palette.js; IE reports it as line 183 (last line in the file), but it's actually line 182. Looks like:

          ...
          reorderSelected : function(movers, before) {
          movers.each(function(option)

          { this.selected.add(option, before); }

          .bind(this));

          this.updateHidden();
          this.updateButtons();
          },
          };

          The culprit is the last comma; IE is apparently unforgiving of trailing commas in map/array definitions.
          Once you do that, the palette loads correctly, and almost functions correctly, too.
          But there's another issue at line 132:

          to.add(option, before);

          An innocuous line. But according to:
          http://www.quirksmode.org/dom/w3c_html.html#selects

          There's some disagreement about the "before" argument among the major browsers.
          W3C says it should be an option object, and mozilla, safari, and opera agree. Mozilla requires the extra argument; safari and opera allow it.
          IE allows the 2nd argument, but expects it to be the /index/ before which to insert the option, rather than an option argument. Hence, the line above generates a "Type mismatch" error in IE.

          I'll try to attach a patch later today.

          Show
          Robert Zeigler added a comment - After some tracking (caveat: only have access to IE 7 at the moment, so there could be further issues on IE 6...), I found two places where IE is choking. 1) in palette.js; IE reports it as line 183 (last line in the file), but it's actually line 182. Looks like: ... reorderSelected : function(movers, before) { movers.each(function(option) { this.selected.add(option, before); } .bind(this)); this.updateHidden(); this.updateButtons(); }, }; The culprit is the last comma; IE is apparently unforgiving of trailing commas in map/array definitions. Once you do that, the palette loads correctly, and almost functions correctly, too. But there's another issue at line 132: to.add(option, before); An innocuous line. But according to: http://www.quirksmode.org/dom/w3c_html.html#selects There's some disagreement about the "before" argument among the major browsers. W3C says it should be an option object, and mozilla, safari, and opera agree. Mozilla requires the extra argument; safari and opera allow it. IE allows the 2nd argument, but expects it to be the /index/ before which to insert the option, rather than an option argument. Hence, the line above generates a "Type mismatch" error in IE. I'll try to attach a patch later today.
          Hide
          Robert Zeigler added a comment -

          1745_patch.txt is a patch to palette.js which removes the extraneous comma after the last palette function definition. It also wraps the "to.add(option,before)" line in a try/catch block, and attempts to.add with an index value for the "before" argument on exception.
          Tested in IE 7, IE 6, Safari 3, and Opera 9.

          Show
          Robert Zeigler added a comment - 1745_patch.txt is a patch to palette.js which removes the extraneous comma after the last palette function definition. It also wraps the "to.add(option,before)" line in a try/catch block, and attempts to.add with an index value for the "before" argument on exception. Tested in IE 7, IE 6, Safari 3, and Opera 9.

            People

            • Assignee:
              Howard M. Lewis Ship
              Reporter:
              Joel Wiegman
            • Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development