Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #11568 (reopened defect)

Opened 1 month ago

Last modified 1 month ago

[PATCH] $ should not throw error when passed a value of an unexistent attribute (IE)

Reported by: gryn Assigned to: sam
Priority: normal Milestone: 2.x
Component: Prototype Version: edge
Severity: normal Keywords:
Cc:

Description

IE (7 tested) seems to be treating some strings more equal than others.

The following example demonstrates: 

<html debug="true">
<head>
  <script type="text/javascript" src="javascripts/prototype.js"></script>
  <script src="javascripts/firebug/firebug.js" type="text/javascript"></script>

  <script type="text/javascript">
Event.observe( window, "load", function() {

  // Expected results:
  // "" :: true
  // undefined
  // undefined

  // "hi" :: true
  // undefined
  // a#hi

  // Actual results:
  // "" :: true
  // undefined
  // Error -- (invalid argument)

  // "hi" :: true
  // undefined
  // a#hi

  ['hi', 'bye'].each( function(id) {
    try {

      console.debug('%o :: %o',
        $(id).target,
        Object.isString( $(id).target ) );

      console.debug( $("") );
      console.debug( $( $(id).target ) );

    } catch(e) {
      console.debug(e);
    }

  });

});
  </script>
</head>
<body>
  <a id="hi">Hello</a>
  <a id="bye" target="hi">Good-bye</a>
</body>
</html>

The error occurs because document.getElementById is being called on what seems to be an empty string (Object.isString says it's a string) but IE considers it invalid. As you can see sending an empty string doesn't cause the same error, just sending it an empty string you got off an attribute that doesn't exist (it's the existence of the attribute -- a blank attribute works without error).

This was found to be not working in Prototype 1.6.0.1. But considering the way the bug works, it's probably present in trunk as well as several previous releases.

Attachments

0013-Should-not-throw-error-when-string-is-malformed.patch (0.6 kB) - added by kangax on 04/13/08 06:19:29.

Change History

04/10/08 17:42:14 changed by gryn

I forgot to mention, the odd thing is this work around: 

  $( $(id).target || "" ); // No Error

04/10/08 20:57:07 changed by kangax

I can confirm: 

document.getElementByID( $(element).target ); 
//throws error if element has no target attribute
// even though "target" is an empty string

readAttribute('target') solves this nicely

04/10/08 21:22:37 changed by kangax

What's interesting is that "target" seems to be a normal string: 

typeof element.target; // "string"
element.target.constructor; // String
element.target === ''; // true
element.target.empty(); // true

Yet, when explicitly typecasted into a string, everything works as expected: 

$(element.target + ''); // => no error

Btw, getElementsByTagName is affected as well.

04/11/08 14:42:46 changed by kangax

  • summary changed from Declaration of String Dependence --or-- Not all strings are created equally --or-- IE throws invalid argument on some strings (Object.isString is not a sufficient check to call document.getElementById). to document.getElementById throws error when passed a value of unexistent attribute (IE).

04/11/08 20:33:33 changed by jdalton

  • status changed from new to closed.
  • resolution set to invalid.

invalid, this is not a bug agianst Prototype.

04/11/08 22:59:23 changed by gryn

Are you sure this is invalid? You don't think that this would be considered buggy code? 

if( type check x )
  function call x

And a type exception is then thrown?

Because that is exactly the code that prototype has written: it's checking to see if something is a string, then calling a function that expects a string. If you aren't going to type check fool-proofly, then you should not have the type check at all, or at least you should catch the (type) exception. No?

I'll leave it up to you, I'm happy enough with my work-around: 

  $( $(id).target || "" )

04/12/08 00:18:33 changed by jdalton

  • status changed from closed to reopened.
  • resolution deleted.
  • summary changed from document.getElementById throws error when passed a value of unexistent attribute (IE) to Object.isString bug: document.getElementById throws error when passed a value of unexistent attribute (IE).

Ahh I see.

04/12/08 00:22:26 changed by kangax

I don't see a way to predict that certain string value throws error when passed to getElementById. As I mentioned before, this "malformed" string does not seem to be recognizable - its typeof, constructor, etc. - all return correct results.

04/12/08 00:35:36 changed by gryn

Why not check, if it says it's a string, then additionally use my work around? 

if( Object.isString( val ) )
  document.getElementById( val || "" );

This is completely benign, because only an non-existent attribute causes the error, and it returns what seems to be an empty string. An empty string is considered false (amongst other non-string like values), and so replacing it with another empty string is fine here. It's not possible after the type check to accidentally change false to an empty string by using the work around.

You could even remove the type check if you are OK with passing false as empty string.

04/12/08 00:53:53 changed by kangax

Well, the check - Object.isString - is already in the code, so I would vote for either: 

getElementById(element + '');
// or
getElementById(element || '');

The first one ensures typecasting (toString) happens no matter what, while the latter one affects empty strings only (but should be faster). Are you sure this happens with empty strings only?

04/12/08 01:18:54 changed by gryn

If you are asking, does IE throw this error any other case, then my answer is I don't know of any other case. That's not to say I'm sure at all. All I know is that I can get these funny mutant-strings from attribute accessors in IE and they only seem to occur when an attribute is not present.

I can't imagine any other case where that would happen, though I'm by no means an IE javascript expert (that's why I use prototype rather than write it ;) ).

I think either check is fine, too.

04/12/08 02:06:24 changed by kangax

  • summary changed from Object.isString bug: document.getElementById throws error when passed a value of unexistent attribute (IE) to $ should not throw error when passed a value of an unexistent attribute (IE).

04/13/08 06:19:29 changed by kangax

  • attachment 0013-Should-not-throw-error-when-string-is-malformed.patch added.

04/13/08 06:19:40 changed by kangax

  • summary changed from $ should not throw error when passed a value of an unexistent attribute (IE) to [PATCH] $ should not throw error when passed a value of an unexistent attribute (IE).

04/13/08 13:28:15 changed by gryn

Someone mentioned getElementsByTagName is also affected, want to fix it too? (It's probably more contrived to make the error happen there.