onClick not working in IE

Discussion in 'Javascript' started by Nx, Mar 15, 2006.

  1. Nx

    Nx Guest

    i've got it all working nicely in firefox, but whenever i test it in IE
    none of the onclick events are triggered.

    i'm using an xsl to transform an rss feed into a photogallery.

    when i try to use setAttribute FF and safari work, but IE stops working
    when i used addEventListener and attachEvent safari stops working
    and when i tried .onClick,none of them worked

    being fairly inexperienced (but learning fast), i figure i'm doing it
    wrong
    any help is appreciated

    snippet:

    <![CDATA[for (var i = 0; i <= (the_news.categories.list.length-1);
    i++){]]>
    if(the_news.categories.list!="News"){
    tempCatDiv = document.createElement("div");
    tempCatDiv.setAttribute('class','catitem');
    tempCatDiv.setAttribute('onClick','showItems("'+the_news.categories.list+'")');
    tempCatDiv.innerHTML = "· "+the_news.categories.list+"";
    catListDiv.appendChild(tempCatDiv);
    }
    }
     
    Nx, Mar 15, 2006
    #1
    1. Advertising

  2. Nx

    web.dev Guest

    Nx wrote:
    > tempCatDiv = document.createElement("div");
    > tempCatDiv.setAttribute('class','catitem');
    > tempCatDiv.setAttribute('onClick','showItems("'+the_news.categories.list+'")');


    IE's setAttribute method is far from perfect. Try setting those
    properties directly:

    tempCatDiv.class = "catitem";
    tempCatDiv.onclick = function() {
    showItems(the_news.categories.list); };
     
    web.dev, Mar 15, 2006
    #2
    1. Advertising

  3. Nx

    Nx Guest

    thanks, i was formatting the onclick badly the first time, but fixing
    it created another problem

    tempCatDiv.onclick = function() {
    showItems(the_news.categories.list);
    };

    passes 'undefined' to showItems

    can you give a clueless coder another freebie?
     
    Nx, Mar 15, 2006
    #3
  4. Nx

    RobG Guest

    Nx said on 16/03/2006 7:00 AM AEST:
    > i've got it all working nicely in firefox, but whenever i test it in IE
    > none of the onclick events are triggered.
    >
    > i'm using an xsl to transform an rss feed into a photogallery.
    >
    > when i try to use setAttribute FF and safari work, but IE stops working
    > when i used addEventListener and attachEvent safari stops working
    > and when i tried .onClick,none of them worked


    JavaScript is case sensitive, the property you seek is 'onclick'.


    > being fairly inexperienced (but learning fast), i figure i'm doing it
    > wrong
    > any help is appreciated
    >
    > snippet:
    >
    > <![CDATA[


    Are you really using XHTML? There seems very little point to it.

    > for (var i = 0; i <= (the_news.categories.list.length-1);
    > i++){]]>


    Don't use tabs, use spaces for indenting code. Manually wrap your code
    for posting at about 70 characters to allow for a coupe of quoted
    replies before autowrapping takes over.

    Why is just the opening - for - statement quoted XHTML-style? Use HTML
    and ditch quoting inside script elements. With XHTML, use an external
    ..js file.

    It is also more efficient to store values that are frequently referenced
    in (local?) variables - the more dots used, the less efficient it becomes:

    for (var i=0, j=the_news.categories.list.length; i<j; i++){


    > if(the_news.categories.list!="News"){


    Since the_news.categories.list is used multiple times, create a local
    variable that references it to save looking it up every time:

    var newsItem = the_news.categories.list;
    if ('News' != newsItem){

    > tempCatDiv = document.createElement("div");
    > tempCatDiv.setAttribute('class','catitem');
    > tempCatDiv.setAttribute('onClick','showItems("'+the_news.categories.list+'")');


    WebDev's suggestion here to use:

    tempCatDiv.onclick = function() {
    showItems(the_news.categories.list);
    };


    creates another problem: the value of - the_news.categories.list - is
    evaluated when the onclick function executes. At that point, if
    the_news.categories.list still exists as a global variable or closure to
    a local variable, then the value of the_news.categories.list will be
    evaluated.

    Since 'i' will also be either a global or closure to a local 'i' inside
    whatever function is running here, it too will be evaluated when the
    onclick occurs.

    That is, the_news.categories.list will be evaluated when the onclick
    code is executed, not when the onclick function object was created.

    At that time, (if it hasn't been modified elsewhere) the value of 'i'
    will be what it was when the for loop finished. It will be the same as
    the_news.categories.list.length (whatever that was).

    Since the value of the length attribute of a collection or array
    (guessing that the_news.categories.list is one or the other) is always
    at least one higher than the largest index, the_news.categories.list
    does not exist.

    e.g. if there were 5 of them, the indexes range from 0 to 4. When the
    for loop finishes, i=5 - all the onclick functions will try to access
    the_news.categories.list[5] which doesn't exist.

    The simple fix is, since the value is always 'News', to use that:

    tempCatDiv.onclick = function(){ showItems('News'); };

    Since the value 'News' is fixed (and essentially hard-coded in your
    current function anyway), why not hard-code it in showItems() and then
    you can have:

    tempCatDiv.onclick = showItems; // Note no '()'


    and showItems is now something like:

    function showItems(){
    var x = 'News';
    ...
    }


    Another, probably less efficient solution but more general, is to use
    (with the substitution of newsItem):

    tempCatDiv.onclick =
    new Function('showItems("'+ newsItem + '"];');


    > tempCatDiv.innerHTML = "· "+the_news.categories.list+"";


    And:
    tempCatDiv.innerHTML = "· " + newsItem;

    > catListDiv.appendChild(tempCatDiv);
    > }
    > }
    >




    --
    Rob
     
    RobG, Mar 16, 2006
    #4
  5. Nx

    RobG Guest

    RobG said on 16/03/2006 10:35 AM AEST:
    > Nx said on 16/03/2006 7:00 AM AEST:

    [...]
    >
    >> if(the_news.categories.list!="News"){

    >
    >
    > Since the_news.categories.list is used multiple times, create a local
    > variable that references it to save looking it up every time:
    >
    > var newsItem = the_news.categories.list;
    > if ('News' != newsItem){

    [...]
    > The simple fix is, since the value is always 'News', to use that:
    >
    > tempCatDiv.onclick = function(){ showItems('News'); };


    Oops! Forgot the '!' the value of newsItem will *not* be 'News' - use
    the suggestion below.


    [...]
    >
    > tempCatDiv.onclick =
    > new Function('showItems("'+ newsItem + '"];');
    >




    --
    Rob
     
    RobG, Mar 16, 2006
    #5
  6. web.dev wrote:

    > Nx wrote:
    >> tempCatDiv = document.createElement("div");
    >> tempCatDiv.setAttribute('class','catitem');
    >>

    tempCatDiv.setAttribute('onClick','showItems("'+the_news.categories.list+'")');
    >
    > IE's setAttribute method is far from perfect.


    Element::setAttribute() has many buggy implementations.

    > Try setting those properties directly:


    Full ACK.

    > tempCatDiv.class = "catitem";


    It is `className', which is well-known here. `class' is a reserved
    word in several languages, including ECMAScript implementations.


    PointedEars
     
    Thomas 'PointedEars' Lahn, Mar 17, 2006
    #6
  7. Nx wrote:

    > thanks, i was formatting the onclick badly the first time, but fixing
    > it created another problem
    >
    > tempCatDiv.onclick = function() {
    > showItems(the_news.categories.list);
    > };
    >
    > passes 'undefined' to showItems


    If the_news.categories.list evaluates to a different
    value, it should not. The error must be elsewhere.

    > can you give a clueless coder another freebie?


    You could RTFM for a change.


    PointedEars
     
    Thomas 'PointedEars' Lahn, Mar 17, 2006
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. moondaddy

    onclick event not working in Netscape 6

    moondaddy, May 1, 2004, in forum: ASP .Net
    Replies:
    5
    Views:
    1,427
    J'son
    May 1, 2004
  2. EMW
    Replies:
    1
    Views:
    358
  3. bob
    Replies:
    3
    Views:
    237
  4. Replies:
    2
    Views:
    280
  5. Replies:
    4
    Views:
    210
Loading...

Share This Page