#480 ✓resolved
Bruce Harris

new Element('select') doesn't display properly in IE6

Reported by Bruce Harris | December 5th, 2008 @ 07:12 PM | in 1.7

When a select element is created using Prototype's Element constructor, it doesn't display properly in IE6 (inconsistent in IE7) when populated dynamically. The markup appears correct if inspected, but the behavior of the element disagrees with the markup (the options usually appear unchanged when dynamically changed, occasionally they change). This is in contrast to a select element created using document.createElement('select') which works fine.

The following code assumes the presence of a div tag with id 'foo'.


var domSelect = document.createElement('select');
var protSelect = new Element('select');

var x = new function(){
	this.populate = function(elements){
		if (!this.counter) this.counter = 0;
		this.counter++;
		elements.each(function(el){
			el.innerHTML = '';
			for (var i = 0; i < 5; i++) {
				var opt = document.createElement('option');
				el.appendChild(opt);
				opt.innerHTML = this.counter;
			}
		}.bind(this));
	}.bind(this);
};

Event.observe(window, 'load', function(){
	var foo = $('foo');
	var btn = new Element('input', {type: 'button', value: 'refresh'});
	foo.appendChild(btn);
	foo.appendChild(domSelect);
	foo.appendChild(protSelect);
	Event.observe(btn, 'click', function(){
		x.populate([domSelect, protSelect]);
		alert(foo.innerHTML);
	});
});

Comments and changes to this ticket

  • Afshin

    Afshin December 5th, 2008 @ 11:01 PM

    Immediately after your for loop in x.populate, if you add this conditional, it will resolve the problem. The bug seems to be in IE's rendering engine and this fix is clearly nonsensical. But it seems to work.

    
    if (typeof el.options.remove !== 'undefined'){ // fixes an IE rendering bug for select tags
        el.options.add(new Element('option'));
        (function(){el.options.remove(el.options.length - 1);}).defer();
    };
    

    So this is what x would look like:

    
    var x = new function(){
    	this.populate = function(elements){
    		if (!this.counter) this.counter = 0;
    		this.counter++;
    		elements.each(function(el){
    			el.innerHTML = '';
    			for (var i = 0; i < 5; i++) {
    				var opt = document.createElement('option');
    				el.appendChild(opt);
    				opt.innerHTML = this.counter;
    			}
    			if (typeof el.options.remove !== 'undefined'){
    			    el.options.add(new Element('option'));
    			    (function(){el.options.remove(el.options.length - 1);}).defer();
    			};
    		}.bind(this));
    	}.bind(this);
    };
    
  • Juriy Zaytsev

    Juriy Zaytsev December 5th, 2008 @ 11:48 PM

    Why do you need the new statement? Why not just assign to x directly - x.populate = function(){ ... }?

  • Afshin

    Afshin December 6th, 2008 @ 12:08 AM

    Setting it equal to new function(){...} lets you have private members inside the object without having to expose them.

    So, for example, you could have:

    
    var x = {populate: function(){}};
    

    But that wouldn't allow you the freedom to have something like this:

    
    var x = new function(){
    	var priv_member = 0;
    	this.populate = function(){
    		/* use priv_member to do something */
    	};
    };
    

    In any case, it's just a different design pattern and shouldn't reflect on the IE rendering bug.

  • Juriy Zaytsev

    Juriy Zaytsev December 6th, 2008 @ 12:26 AM

    In any case, it's just a different design pattern and shouldn't reflect on the IE rendering bug.

    Of course it shouldn't : )

    It just seemed weird to see this pattern in a context of one assignment and lack of local variables.

    As far as your original problem, does Afshin's example "fix" the rendering?

  • Tobie Langel

    Tobie Langel December 10th, 2008 @ 08:19 PM

    • Assigned user set to “Juriy Zaytsev”
    • State changed from “new” to “bug”
  • Bruce Harris

    Bruce Harris December 10th, 2008 @ 08:39 PM

    In response to Juriy, yes Afshin's example does "fix" the rendering, but then so does using document.createElement() instead of new Element(), so it's a workaround for what seems to be a Prototype bug that happens upon an IE bug.

    With regard to the design pattern, agreed, my example code would have been more straightforward if counter was simply a global, but I'm in the habit of avoiding globals where possible.

  • Juriy Zaytsev

    Juriy Zaytsev December 13th, 2008 @ 06:32 AM

    • Milestone set to 1.7

    A problem seems to be in cloneNode that Element uses internally when a cached version of an element is found.

    Here's a minimal test case (which shows how cloning an element fails in the very same way as when Element is used):

    
    <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN"
       "http://www.w3.org/TR/html4/strict.dtd">
    <html lang="en">
      <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>
        <h2>select element should have 1 option with "test" value</h2>
        <button id="trigger">go</button>
        <script type="text/javascript">
          var sel = new Element('select');
          // var sel = document.createElement('select'); works
          // var sel = document.createElement('select').cloneNode(false); fails
          document.body.appendChild(sel);
          $('trigger').observe('click', function(){
            sel.appendChild(new Element('option').update('test'));
          });
        </script>
      </body>
    </html>
    
  • Juriy Zaytsev

    Juriy Zaytsev December 14th, 2008 @ 05:12 AM

    • Tag changed from element, ie6, ie7, select to element, ie6, ie7, needs_discussion, needs_patch, needs_tests, select

    First thing that comes to mind is to skip cache step when element-to-be-created is a select element.

    Any other ideas?

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:11 AM

    • Tag changed from element, ie6, ie7, needs_discussion, needs_patch, needs_tests, select to element, ie6, ie7, needs_discussion, needs_patch, needs_tests, section:dom

    [not-tagged:"dom" tagged:"section:dom" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:12 AM

    • Tag changed from element, ie6, ie7, needs_discussion, needs_patch, needs_tests, section:dom to ie6, ie7, needs_discussion, needs_patch, needs_tests, section:dom

    [not-tagged:"element" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:26 AM

    • Tag changed from ie6, ie7, needs_discussion, needs_patch, needs_tests, section:dom to ie6, ie7, missing:tests, needs_discussion, needs_patch, section:dom

    [not-tagged:"needs_tests" tagged:"missing:tests" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:28 AM

    • Tag changed from ie6, ie7, missing:tests, needs_discussion, needs_patch, section:dom to ie6, ie7, missing:patch, missing:tests, needs_discussion, section:dom

    [not-tagged:"needs_patch" tagged:"missing:patch" bulk edit command]

  • Tobie Langel

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

    • Tag changed from ie6, ie7, missing:patch, missing:tests, needs_discussion, section:dom to ie6, ie7, missing:patch, needs:tests, needs_discussion, section:dom

    [not-tagged:"missing:tests" tagged:"needs:tests" bulk edit command]

  • Tobie Langel

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

    • Tag changed from ie6, ie7, missing:patch, needs:tests, needs_discussion, section:dom to ie6, ie7, needs:patch, needs:tests, needs_discussion, section:dom

    [not-tagged:"missing:patch" tagged:"needs:patch" bulk edit command]

  • Tobie Langel

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

    • Tag changed from ie6, ie7, needs:patch, needs:tests, needs_discussion, section:dom to ie6, ie7, needs:discussion, needs:patch, needs:tests, section:dom

    [not-tagged:"needs_discussion" tagged:"needs:discussion" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 12:46 PM

    • Tag changed from ie6, ie7, needs:discussion, needs:patch, needs:tests, section:dom to needs:discussion, needs:patch, needs:tests, section:dom

    [not-tagged:"ie6" not-tagged:"ie7" bulk edit command]

  • T.J. Crowder

    T.J. Crowder November 16th, 2009 @ 04:50 PM

    • Assigned user cleared.

    [responsible:none bulk edit command]

  • Andrew Dupont

    Andrew Dupont October 17th, 2010 @ 11:01 PM

    • Importance changed from “” to “”

    I'm going to take the odd step of committing a fix without any unit tests, because:

    1. it isn't clear to me how I can reproduce (in an automated fashion) the problem described;
    2. we're already bypassing the tag cache in other situations, so it was easy to add this to the existing logic.

    (Part of me thinks we should move away from the tag cache altogether. It's a minor optimization at best and it's caused at least two bugs now. But I digress.)

    If someone can figure out a good unit test to guard against this issue, do tell.

  • GitHub Robot

    GitHub Robot October 18th, 2010 @ 12:52 AM

    • State changed from “bug” to “resolved”

    (from [aeb45325d40f4a31c3c3b2687ede876b2c7e3c1c]) Fix odd behavior with new Element('select') in IE6-7. [#480 state:resolved] (Bruce Harris, kangax, Andrew Dupont) http://github.com/sstephenson/prototype/commit/aeb45325d40f4a31c3c3...

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

Referenced by

Pages