IE Version 6.0.2900.2180.xpsp_sp2_gdr.050301-1519

Clicking on the stars then this error.

Bummer dude.

Mike

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MichaelCole’s picture

The Microsoft Script Editor debugger points to one monstrous line of code

MichaelCole’s picture

One last thing... The vote "stays" so if I click 2 stars, I get the error, but the control remembers my vote.

The cancel rating button works ok.

mkjones’s picture

I was getting this yesterday with IE6, however the vote was still case, is that correct?

mkjones’s picture

sorry, I mean: "the vote was still cast"

ivix’s picture

Getting the same error in IE7.

mr700’s picture

I can confirm this, I get:

Line 187 Char 3
Invalid argument (when the node is rendered)
Line 162 Char 8
Object doesn't support this property or method (every time I click something)
mr700’s picture

In jquery.js 1.0.4 (not the packed version) from node 110935 comment 13 at line 1521(1519 in the unpatched version) in find: function( t, context ) there's:

                                                for ( var i = 0; i < ret.length; i++ )
                                                        r = jQuery.merge( r,
                                                                m[2] == "*" ?
                                                                        jQuery.getAll(ret[i]) :
                                                                        ret[i].getElementsByTagName(m[2])
                                                        );

The error occurs on ret[i].getElementsByTagName(m[2]).

mr700’s picture

Status: Active » Needs review
FileSize
695 bytes

I think I figured this out (by trial and error but anyway). There are actually two separate errors, but I don't know how to 'clone' this bug here I'll put two patches.

The first problem is that the 'fix ie6 background flicker problem' is for IE=6 only, not 6.x, so

        // fix ie6 background flicker problem.
        if ($.browser.msie == true) {
                document.execCommand('BackgroundImageCache', false, true);
        }

should probably be

  // fix ie6 background flicker problem.
  if ($.browser.msie == true) {
    try {
      document.execCommand('BackgroundImageCache', false, true);
    } catch(err) {}
  }

or http://www.hedgerwow.com/360/bugs/dom-fix-ie6-background-image-flicker.html is a better idea, haven't tried this one.
NB: This is patch 1/2.

mr700’s picture

And the second one. I have absolutely no idea what and how this code works (almost), but changing '$(data)' with 'data' in voteHook silences my IE6. I've tested this one (with the previous patch applied too) with FF 1.5, Konqueror 3.5.6, Opera 9.10 and IE 6.0.2800.1106.xpsp1.020828-1920 and all they work without any error. Clicking on the stars shows ..../fivestar/vote/node/[nid]/[rating] in my apache logs and the database changes accordingly, so I assume I got lucky and fixed it. Here are the change and the patch:
old code:

       returnObj.result.count = $("count",$(data)).text();
       returnObj.result.average = $("average",$(data)).text();
       returnObj.vote.id = $("id",$(data)).text();
       returnObj.vote.type = $("type",$(data)).text();
       returnObj.vote.value = $("value",$(data)).text();

new code:

       returnObj.result.count = $("count",data).text();
       returnObj.result.average = $("average",data).text();
       returnObj.vote.id = $("id",data).text();
       returnObj.vote.type = $("type",data).text();
       returnObj.vote.value = $("value",data).text();

Would be nice if someone who understands jQuery can verify this...

mr700’s picture

If the previous two patches are ok, here's one that includes them and also removes the extra spaces and makes the code indentation match drupal coding standards (maybe just attaching the new file was a better idea).

jonathan_hunt’s picture

I applied these patches manually. Fivestar now works ok on IE6 and IE7.

b_f’s picture

Yes, the two patches correct the problem. Thanks, Doncho.

quicksketch’s picture

Status: Needs review » Needs work

Thanks mr700! I applied your IE flicker patch and committed to CVS. Unfortunately the second patch (removing $) breaks Firefox 2 (Mac) in my testing. There are like 6 issues open with this same problem, so one of them will probably work :D

quicksketch’s picture

Status: Needs work » Fixed

Okay, both patches were fantastic :)

I combined the second patch with http://drupal.org/node/132666, which properly sets the content type on the returned xml. Now jQuery gets an XMLObject (not html) returned in all browsers. Firefox had been compensating and reading it as XML, but oddly... IE was doing the right thing! First time for that one...

Thanks again mr700. Both are now applied and will be in the 1.5 release.

mr700’s picture

Thank you too.

Dries’s picture

Status: Fixed » Closed (fixed)