Prototype

Comments and changes to this ticket

• 

  John-David Dalton August 4th, 2008 @ 03:02 PM

  Can you provide an example snippet. NM I see the unit test in the patch :)

• 

  John-David Dalton August 4th, 2008 @ 03:04 PM

  • → Milestone changed from “” to “1.6.1”
  • → State changed from “new” to “bug”
  • → Assigned user changed from “” to “John-David Dalton”
• 

  Tobias H. Michaelsen August 4th, 2008 @ 03:10 PM

  Example:

  var css = '<link rel="stylesheet" type="text/css" href="my.css" />';
  $$('head').first().insert(css);
  

• 

  Jesper Rønn-Jensen August 4th, 2008 @ 03:17 PM

  This bug is highly annoying in this typical usecase where you want to add a css file reference via javascript. As i recall, this must be done in the HEAD section of the file and use a LINK element.

  I am attaching a working example with a workaround that uses document.createElement in stead of an html string (but that's far from elegant).

  The example is working code from a current project. It attaches a css file to html head if it's necessary.

  So I really vote in favor that you include this patch as soon as possible in the release cycle.

  • prototype-ticket-264-example.js 773 Bytes
• 

  John-David Dalton August 4th, 2008 @ 04:09 PM

  By the way, thanks for taking the time to make a patch and unit tests.

  This really makes things easier :)

• 

  Tobias H. Michaelsen August 4th, 2008 @ 04:25 PM

  Sorry about making the diff from Subversion :)

  Just had the old repository laying around and didn't think about the move to Git until after I submitted the ticket.

• 

  Tobie Langel August 4th, 2008 @ 04:29 PM

  I'd like to avoid the sniff if that's at all possible.

• 

  John-David Dalton October 5th, 2008 @ 07:07 PM

  Ya it wouldn't hurt to simply remove the sniff and just append the dummy text to everything, or we could do a capability test (perform a one time test and if it fails fork the method).

  -- remove snippet because a patch has been attached --

• 

  Tobias H. Michaelsen August 4th, 2008 @ 08:53 PM

  It isn't really a capability problem but seemingly a bug in IE.

  Setting the innerHTML to something that contains a link element will create every other elements but just ignore the link, unless there is some text before the element -- after will not do.

  var div = new Element('div');
  div.innerHTML = '<link>';
  // => ''
  div.innerHTML = '<div><link></div>';
  // => '<div></div>' 
  div.innerHTML = '<div><link></div>text';
  // => '<div></div>text'
  div.innerHTML = 'text<link>'
  // => 'text<link>'
  

  I was also thinking that maybe it was because a link could only be added to the head element, but trying to set innerHTML on that causes an exception in IE.

  While I'm not crazy about browser sniffing, I do think there should be some kind of indication that this is a IE specific issue. That said, I don't think adding the wrapper as default should cause any problems in other browser.

• 

  Juriy Zaytsev August 4th, 2008 @ 08:57 PM

  Appending text to everything is silly : )

  Capability test should do just fine.

• 

  Tobias H. Michaelsen August 4th, 2008 @ 09:32 PM

  Testing for the buggy parsing behavior would result in a lot of overhead and extra code.

  I've attached an alternative patch (w/o the test) that just always sets a wrapper, but still uses the sniffer (but that could easily be omitted).

  • 0001-Alt.-patch-for-buggy-IE-innerHTML-parsing.patch 1.1 KB
• 

  Tobias H. Michaelsen August 4th, 2008 @ 09:36 PM

  JDD: Didn't see your post before I wrote mine. I'm still not sure which approach I prefer.

• 

  Juriy Zaytsev August 4th, 2008 @ 09:37 PM

  @JDD

  Oh, my bad :)

  @Tobias

  The code size decrease is not really worth method reliability.