Posted by MichaelCole on January 19, 2007 at 8:12am
| Project: | Fivestar |
| Version: | 5.x-1.3 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
IE Version 6.0.2900.2180.xpsp_sp2_gdr.050301-1519
Clicking on the stars then this error.
Bummer dude.
Mike
Comments
#1
The Microsoft Script Editor debugger points to one monstrous line of code
#2
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.
#3
I was getting this yesterday with IE6, however the vote was still case, is that correct?
#4
sorry, I mean: "the vote was still cast"
#5
Getting the same error in IE7.
#6
I can confirm this, I get:
#7
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]).
#8
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
<?php// fix ie6 background flicker problem.
if ($.browser.msie == true) {
document.execCommand('BackgroundImageCache', false, true);
}
?>
should probably be
<?php// 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.
#9
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...
#10
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).
#11
I applied these patches manually. Fivestar now works ok on IE6 and IE7.
#12
Yes, the two patches correct the problem. Thanks, Doncho.
#13
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
#14
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.
#15
Thank you too.
#16