Bug 8547 - Enabling star ratings causes javascript errors that cause IE to have a boo boo
Summary: Enabling star ratings causes javascript errors that cause IE to have a boo boo
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Browser compatibility (show other bugs)
Version: 3.8
Hardware: All All
: P2 major (vote)
Assignee: Robin Sheat
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-01 12:18 UTC by Robin Sheat
Modified: 2013-12-05 19:59 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8547 - JS error in star ratings breaks IE (1005 bytes, patch)
2012-08-01 12:56 UTC, Robin Sheat
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8547 - JS error in star ratings breaks IE (1.18 KB, patch)
2012-08-04 22:00 UTC, Marc Véron
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Sheat 2012-08-01 12:18:30 UTC
I think this is due to a trailing comma in a list. Real browser are OK with it, but IE is dumb.
Comment 1 Robin Sheat 2012-08-01 12:53:44 UTC
Seeing this issue seems to be a bit hit and miss, I haven't worked out the combination of parameters that causes it, just that sometimes it'll happen and sometimes it won't (even with the same browser.) I suspect it's to do with the order that certain ajax loads take place relative to JS rendering.
Comment 2 Robin Sheat 2012-08-01 12:56:24 UTC Comment hidden (obsolete)
Comment 3 Robin Sheat 2012-08-01 13:02:07 UTC
This patch is based on a hunch of what the issue is and is still waiting for someone who has IE to test it for me, but if anyone else is interested, here's what it looks like when it's going bad:

http://i.imgur.com/Jhc3z.jpg
Comment 4 Owen Leonard 2012-08-01 13:38:27 UTC
For what it's worth, I don't see this problem in IE9 on Windows 7.
Comment 5 Katrin Fischer 2012-08-01 14:04:28 UTC
I was testing IE 8 on Windows 7 and saw it sometimes. Most of the time it happened after closing the browser completely and going to the detail page directly following a link from my bookmarks.
Comment 6 Robin Sheat 2012-08-02 12:20:42 UTC
Confirmed from client that this solves the issue.
Comment 7 Marc Véron 2012-08-04 20:34:32 UTC
Bug confirmeed with IE 7.0.5730.13 on Windows XP
Comment 8 Marc Véron 2012-08-04 22:00:34 UTC
Created attachment 11361 [details] [review]
[SIGNED-OFF] Bug 8547 - JS error in star ratings breaks IE

Writing code with best practices in mind breaks IE. Thanks Internet
Explorer. Now go DiaF.

Signed-off-by: Marc Veron <veron@veron.ch>

Tested with with IE 7.0.5730.13 on Windows XP, no JS errors, stars display and behave as expected
Comment 9 Jonathan Druart 2012-08-06 08:28:29 UTC
QA Comments:

This patch fixes a "Trailing comma of death" (see http://www.enterprisedojo.com/2010/12/19/beware-the-trailing-comma-of-death/) for IE

Passed QA
Comment 10 Chris Cormack 2012-08-06 08:36:24 UTC
Have picked this into 3.8.x, will be in 3.8.4, safe change, fixes broken IE. (Well fixes one small symptom of its huge pile of brokenness anyway)

Leaving as Passed QA, so Paul can push to master when he gets back.
Comment 11 Paul Poulain 2012-08-29 13:17:33 UTC
Patch pushed today (checked it does not break FF)