STR: Visit https://preview.addons.mozilla.org/en-US/firefox/collections/editors_picks whiles logged in 2) Click on any addon to rate 3) Click on any addons add to favorites button Expected Results: 1) User is able to add a favorite 2) User is able to rate addon Actual Results: 1)User is unable to rate an addon / add a favorite Screencast attached http://screencast.com/t/AI59QOQg
Assignee: nobody → sancus
Status: NEW → ASSIGNED
I couldn't reproduce the voting not working -- in both IE7/8 it worked fine for me. So either that was randomly fixed, or it's something more subtle than just JS that's incompatible-with-IE. This patch fixes Add To Favorite, which errored in two places, one because b.css('width') == 'auto' under IE, not a value. The jQuery .width() method seems to work fine under both IE and Firefox, so I just switched to using that. All the other CSS values seem valid under both.
Attachment #409992 - Flags: review?(fwenzel)
Comment on attachment 409992 [details] [diff] [review] IE 'Add Favorite' Fix Looks good! r55159, thanks, Sancus!
Attachment #409992 - Flags: review?(fwenzel) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Voting works on IE 7/8 but 'Add to Favorites' does not. This is the error detail when you click 'Add to Favorites': Webpage error details User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0) Timestamp: Fri, 6 Nov 2009 19:18:32 UTC Message: Invalid argument. Line: 1 Char: 12974 Code: 0 URI: https://preview.addons.mozilla.org/js/amo2009/amo2009.min.js?55160
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Webpage error details User Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30; .NET CLR 3.0.04506.648; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729) Timestamp: Fri, 6 Nov 2009 19:38:26 UTC Message: 'JSON' is undefined Line: 137 Char: 13 Code: 0 URI: http://jbalogh.khan.mozilla.org/amo/site/js/amo2009/collections.js
The bad line is in a function that won't get executed unless we have localStorage, so I guess IE is trying to pre-parse everything and then bailing?
Are you getting that error under IE7, IE8, or both? I get a different error in IE8(though still an error), and my Windows VM appears screwed up and IE7 refuses to report errors no matter what I check in Internet Options. I'll get this re-fixed as soon as I straighten out my VM.
(In reply to comment #6) > Are you getting that error under IE7, IE8, or both? Just IE7. IE8 has native JSON, so there wouldn't be a problem.
Couple things here: The whole maintain_width function is replaceable by a jQuery function, so I removed that. The jQuery function comes with the side benefit of not erroring on IE due to NaN css properties, and produces the exact same sum testing on Firefox with Firebug as the function it replaced. Secondly, the JSON issue seems to be intermittent and only happens when I'm logged in. I guess the recently viewed stuff is trying to function. I put in a couple of checks to make sure JSON is defined before calling methods, as that prevents the whole thing from exploding under IE7. If we want that to actually work properly under IE7 I guess we'd have to provide a JSON implementation for it, but I think that's a bit beyond the scope of this bug.
Comment on attachment 411308 [details] [diff] [review] IE 'Add Favorite' Fix v2 I like most of this patch, but I don't like that we just don't use JSON when the browser doesn't natively support it -- that means it won't work in Fx2 either. Instead, we should just import json.js which is the JSON implementation from http://json.org -- if needed, we should probably update it to the latest version -- it'll only load its JSON implementation when it's not supported natively, which is exactly what we want.
Attachment #411308 - Flags: review?(fwenzel) → review-
(it's probably worth mentioning that we already have json.js in our js directory and are using it on the developer pages).
OK, I added json.js to any place that depends on collections.js, this should solve the intermittent JSON undefined bugs, and also prevent them anywhere else. Thanks wenzel!
Why is json.js getting packer'd? That's lame.
Comment on attachment 411331 [details] [diff] [review] IE 'Add Favorite' Fix v3 Looks good, but we'll need to double-check once it's on preview -- I am unsure if we need to add it to the build script, too.
Attachment #411331 - Flags: review?(fwenzel) → review+
(In reply to comment #12) > Why is json.js getting packer'd? That's lame. You could go ahead and upload the unpacked version instead, as well as adding it to the build script.
I don't think it needs to be added to the build script. Unless I'm completely wrong(which is possible), it looks like there's a default set of .js that is minified, or echo'd explicitly if the site is in a dev state. The per-controller and per-page jsAdds are still respected regardless of site state, so unless you want to add json.js to ALL pages site-wide(in which case my patch is wrong, and jsAdd is not necessary) then I don't think it belongs in the build script. The reason it's packer'd is probably because of the above.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago → 10 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.