Tapestry
  1. Tapestry
  2. TAPESTRY-1206

Asynchronous Selection Boxes work with Firefox 2.0 but not with Internet Explorer 6/7

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1.1
    • Fix Version/s: 4.1.2
    • Component/s: XHR/dhtml/Ajax
    • Labels:
      None
    • Environment:
      We are using the 4.1.1 snapshots of tapestry

      Description

      We have discovered that the use of dependency in selection boxes leads to problems, if you use asynchron calls and internet explorer.

      example files are attached below

      1. screenshot_ie6.jpg
        85 kB
        Christian Köberl
      2. selectAjaxUpdate.zip
        4 kB
        Christian Köberl
      3. SelectionTest.html
        0.7 kB
        Stefan Esterer
      4. SelectionTest.java
        1 kB
        Stefan Esterer
      5. SelectionTest.page
        0.7 kB
        Stefan Esterer
      6. TAPESTRY-1206.patch
        0.9 kB
        Christian Köberl

        Activity

        Hide
        Christian Köberl added a comment -

        The Maven/Eclipse project in selectAjaxUpdate.zip makes it easier to reconstruct the problem. I tried to find the error - it seems that Dojo in connection with IE update the DOM tree in some weird way (see screenshot_i6.jpg). The first open-tag for the options is not added. I couldn't find the source of this problem. Maybe someone has an idea!?

        Show
        Christian Köberl added a comment - The Maven/Eclipse project in selectAjaxUpdate.zip makes it easier to reconstruct the problem. I tried to find the error - it seems that Dojo in connection with IE update the DOM tree in some weird way (see screenshot_i6.jpg). The first open-tag for the options is not added. I couldn't find the source of this problem. Maybe someone has an idea!?
        Hide
        Christian Köberl added a comment -

        Seems like that this browser bug in IE is still not fixed (even in IE7 - I hate those MS guys)
        http://support.microsoft.com/default.aspx?scid=kb;en-us;276228

        This causes the problem because the SELECT boxes are updated via innerHTML.

        Show
        Christian Köberl added a comment - Seems like that this browser bug in IE is still not fixed (even in IE7 - I hate those MS guys) http://support.microsoft.com/default.aspx?scid=kb;en-us;276228 This causes the problem because the SELECT boxes are updated via innerHTML.
        Hide
        Christian Köberl added a comment -

        The problem is in core.js of tapestry-framework. Here the innerHTML is used to update a node in the dom tree.

        I did a simple workaround for MSIE. It works for IE6 on XP and it does not change the behavior of other browsers. I have to test in in IE7. Here is the patch:

        Index: src/js/tapestry/core.js
        ===================================================================
        — src/js/tapestry/core.js (revision 497116)
        +++ src/js/tapestry/core.js (working copy)
        @@ -196,12 +196,15 @@

        dojo.event.browser.clean(node); // prevent mem leaks in ie

        + var content=tapestry.html.getContentAsString(element);
        if (djConfig["isDebug"])

        { - var content=tapestry.html.getContentAsString(element); dojo.log.debug("Received element content for id <" + id + "> of:", content); + }

        + // fix for IE - setting innerHTML does not work for SELECTs
        + if (dojo.render.html.ie && node.outerHTML && node.nodeName == "SELECT")

        { + node.outerHTML=node.outerHTML.replace(/<OPTION.*OPTION>/, content); + }

        else

        { node.innerHTML=content; - }

        else

        { - node.innerHTML=tapestry.html.getContentAsString(element); }

        // copy style/class css attributes

        Show
        Christian Köberl added a comment - The problem is in core.js of tapestry-framework. Here the innerHTML is used to update a node in the dom tree. I did a simple workaround for MSIE. It works for IE6 on XP and it does not change the behavior of other browsers. I have to test in in IE7. Here is the patch: Index: src/js/tapestry/core.js =================================================================== — src/js/tapestry/core.js (revision 497116) +++ src/js/tapestry/core.js (working copy) @@ -196,12 +196,15 @@ dojo.event.browser.clean(node); // prevent mem leaks in ie + var content=tapestry.html.getContentAsString(element); if (djConfig ["isDebug"] ) { - var content=tapestry.html.getContentAsString(element); dojo.log.debug("Received element content for id <" + id + "> of:", content); + } + // fix for IE - setting innerHTML does not work for SELECTs + if (dojo.render.html.ie && node.outerHTML && node.nodeName == "SELECT") { + node.outerHTML=node.outerHTML.replace(/<OPTION.*OPTION>/, content); + } else { node.innerHTML=content; - } else { - node.innerHTML=tapestry.html.getContentAsString(element); } // copy style/class css attributes
        Hide
        David Davis added a comment -

        One note on the above patch:
        {{

        { node.outerHTML=node.outerHTML.replace(/<OPTION.*OPTION>/, content); }

        }}

        That line only works if at least one OPTION is initially present. So it doesn't work (on IE6) if the HTML is initialized as:
        {{

        { <SELECT id="foo"> </SELECT> }

        }}

        Otherwise, the patch worked for me on IE6/Linux.

        Show
        David Davis added a comment - One note on the above patch: {{ { node.outerHTML=node.outerHTML.replace(/<OPTION.*OPTION>/, content); } }} That line only works if at least one OPTION is initially present. So it doesn't work (on IE6) if the HTML is initialized as: {{ { <SELECT id="foo"> </SELECT> } }} Otherwise, the patch worked for me on IE6/Linux.
        Hide
        Christian Köberl added a comment -

        @Dave:
        the regular expression replace should work anyway because it should not match anything when no options are present. At least it works for me IE6/WinXP.
        You have an IE6 running on Linux?!

        But your comment made me think about the patch and I realized the there could also be <OPTGROUP> elements in there. So, the replace should be:
        node.outerHTML=node.outerHTML.replace(/<OPT(ION|GROUP).*OPT(ION|GROUP)>/, content);

        Here's the new patch:

        Index: src/js/tapestry/core.js
        ===================================================================
        — src/js/tapestry/core.js (revision 497116)
        +++ src/js/tapestry/core.js (working copy)
        @@ -196,12 +196,15 @@

        dojo.event.browser.clean(node); // prevent mem leaks in ie

        + var content=tapestry.html.getContentAsString(element);
        if (djConfig["isDebug"])

        { - var content=tapestry.html.getContentAsString(element); dojo.log.debug("Received element content for id <" + id + "> of:", content); + }

        + // fix for IE - setting innerHTML does not work for SELECTs
        + if (dojo.render.html.ie && node.outerHTML && node.nodeName == "SELECT")

        { + node.outerHTML=node.outerHTML.replace(/<OPT(ION|GROUP).*OPT(ION|GROUP)>/, content); + }

        else

        { node.innerHTML=content; - }

        else

        { - node.innerHTML=tapestry.html.getContentAsString(element); }

        // copy style/class css attributes

        Show
        Christian Köberl added a comment - @Dave: the regular expression replace should work anyway because it should not match anything when no options are present. At least it works for me IE6/WinXP. You have an IE6 running on Linux?! But your comment made me think about the patch and I realized the there could also be <OPTGROUP> elements in there. So, the replace should be: node.outerHTML=node.outerHTML.replace(/<OPT(ION|GROUP).*OPT(ION|GROUP)>/, content); Here's the new patch: Index: src/js/tapestry/core.js =================================================================== — src/js/tapestry/core.js (revision 497116) +++ src/js/tapestry/core.js (working copy) @@ -196,12 +196,15 @@ dojo.event.browser.clean(node); // prevent mem leaks in ie + var content=tapestry.html.getContentAsString(element); if (djConfig ["isDebug"] ) { - var content=tapestry.html.getContentAsString(element); dojo.log.debug("Received element content for id <" + id + "> of:", content); + } + // fix for IE - setting innerHTML does not work for SELECTs + if (dojo.render.html.ie && node.outerHTML && node.nodeName == "SELECT") { + node.outerHTML=node.outerHTML.replace(/<OPT(ION|GROUP).*OPT(ION|GROUP)>/, content); + } else { node.innerHTML=content; - } else { - node.innerHTML=tapestry.html.getContentAsString(element); } // copy style/class css attributes
        Hide
        Christian Köberl added a comment -

        Oh, I see: the regular expression does not fill in the new element, when it's empty (sometimes I need some time )

        Show
        Christian Köberl added a comment - Oh, I see: the regular expression does not fill in the new element, when it's empty (sometimes I need some time )
        Hide
        Christian Köberl added a comment -

        OK, now I think I've got it.

        This replace should do it correctly:
        node.outerHTML = selectOuterHtml.replace(/(<SELECT[^<]>).(<\/SELECT>)/, '$1' + content + '$2');

        I've tested it with IE6/7 on WinXP and Firefox 2.0.0.1 on WinXP.
        Test cases were:
        1. empty SELECT
        2. SELECT with OPTIONs
        3. SELECT with OPTGROUPs and OPTIONs

        Here's the (hopefully final) patch:

        Index: src/js/tapestry/core.js
        ===================================================================
        — src/js/tapestry/core.js (revision 497116)
        +++ src/js/tapestry/core.js (working copy)
        @@ -196,12 +196,15 @@

        dojo.event.browser.clean(node); // prevent mem leaks in ie

        + var content=tapestry.html.getContentAsString(element);
        if (djConfig["isDebug"])

        { - var content=tapestry.html.getContentAsString(element); dojo.log.debug("Received element content for id <" + id + "> of:", content); + }

        + // fix for IE - setting innerHTML does not work for SELECTs
        + if (dojo.render.html.ie && node.outerHTML && node.nodeName == "SELECT")

        { + node.outerHTML = selectOuterHtml.replace(/(<SELECT[^<]*>).*(<\/SELECT>)/, '$1' + content + '$2'); + }

        else

        { node.innerHTML=content; - }

        else

        { - node.innerHTML=tapestry.html.getContentAsString(element); }

        // copy style/class css attributes

        Show
        Christian Köberl added a comment - OK, now I think I've got it. This replace should do it correctly: node.outerHTML = selectOuterHtml.replace(/(<SELECT [^<] >). (<\/SELECT>)/, '$1' + content + '$2'); I've tested it with IE6/7 on WinXP and Firefox 2.0.0.1 on WinXP. Test cases were: 1. empty SELECT 2. SELECT with OPTIONs 3. SELECT with OPTGROUPs and OPTIONs Here's the (hopefully final) patch: Index: src/js/tapestry/core.js =================================================================== — src/js/tapestry/core.js (revision 497116) +++ src/js/tapestry/core.js (working copy) @@ -196,12 +196,15 @@ dojo.event.browser.clean(node); // prevent mem leaks in ie + var content=tapestry.html.getContentAsString(element); if (djConfig ["isDebug"] ) { - var content=tapestry.html.getContentAsString(element); dojo.log.debug("Received element content for id <" + id + "> of:", content); + } + // fix for IE - setting innerHTML does not work for SELECTs + if (dojo.render.html.ie && node.outerHTML && node.nodeName == "SELECT") { + node.outerHTML = selectOuterHtml.replace(/(<SELECT[^<]*>).*(<\/SELECT>)/, '$1' + content + '$2'); + } else { node.innerHTML=content; - } else { - node.innerHTML=tapestry.html.getContentAsString(element); } // copy style/class css attributes
        Hide
        David Davis added a comment -

        I tested the above patch modification, and it works for me:
        {{

        { node.outerHTML = selectOuterHtml.replace(/(<SELECT[^<]*>).*(<\/SELECT>)/, '$1' + content + '$2'); }

        }}

        Also, FYI: IE on Linux is possible through ies4linux: http://www.tatanka.com.br/ies4linux/

        Show
        David Davis added a comment - I tested the above patch modification, and it works for me: {{ { node.outerHTML = selectOuterHtml.replace(/(<SELECT[^<]*>).*(<\/SELECT>)/, '$1' + content + '$2'); } }} Also, FYI: IE on Linux is possible through ies4linux: http://www.tatanka.com.br/ies4linux/
        Hide
        Jesse Kuhnert added a comment -

        Thanks for all the hard work on this ! (and the link to ies4linux - had no idea someone had something done for this in wine)

        Show
        Jesse Kuhnert added a comment - Thanks for all the hard work on this ! (and the link to ies4linux - had no idea someone had something done for this in wine)

          People

          • Assignee:
            Jesse Kuhnert
            Reporter:
            Stefan Esterer
          • Votes:
            5 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development