Bug 12325 - Marc and Card view in Z39.50 Results doesn't work properly in IE 8
Summary: Marc and Card view in Z39.50 Results doesn't work properly in IE 8
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Z39.50 / SRU / OpenSearch Servers (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-29 07:37 UTC by David Cook
Modified: 2015-06-04 23:33 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12325 - Marc and Card view in Z39.50 Results doesn't work properly in IE 8 (3.98 KB, patch)
2014-05-30 07:06 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12325 - Marc and Card view in Z39.50 Results doesn't work properly in IE 8 (4.08 KB, patch)
2014-08-13 15:52 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 12325 - Marc and Card view in Z39.50 Results doesn't work properly in IE 8 (4.22 KB, patch)
2014-08-15 17:29 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2014-05-29 07:37:03 UTC
It would appear that Bug 5184 has introduced incompatibilities with IE 8 (and likely older versions of IE). 

Bug 5184 - Upgrade jQuery to the latest version (staff client)
commit d1e3773e3b6bc83e4e7937b3c79c886770bc2c53

I found it using the following command "git bisect start v3.10.00 v3.08.00". All hail git bisect!

The Greybox libraries are the same, and they're called in the same way, so I'm thinking that jQuery has introduced some code that gets loaded and causes problems in the background. To date, usually these errors have manifested quite readily in the browser so they're easy to address, but this one is rather...insidious.

Anyway, I don't expect folks to care too much, but it's worth logging as a fact.

I'll look into a fix, but...if it's possible to fix it...it's going to probably be with a hack...
Comment 1 David Cook 2014-05-30 07:06:28 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2014-07-17 12:20:05 UTC
But do we or even should we support IE8 for the staff client?
Comment 3 David Cook 2014-07-30 01:02:21 UTC
(In reply to M. de Rooy from comment #2)
> But do we or even should we support IE8 for the staff client?

I don't see why not.

We don't "officially" support any browser. While IE isn't popular amongst developers, it's used out in the real world. While I might be one of the few willing to report and fix bugs that surface for IE users, I don't think that makes the patches any less useful or desirable for the Koha codebase.

If I were adding an IE-specific hack, I could understand some reluctance to include it into the codebase. However, in this case, I'm not adding hacks, doing browser detection, or even feature detection (which I think is actually pretty good to do anyway). I'm removing a line of code that probably shouldn't be there regardless of what browser is executing it.

I'm also fixing the display of in-line links for all browser, although I suppose I could've done a separate patch for that.
Comment 4 Katrin Fischer 2014-08-04 19:23:23 UTC
I am happy to see IE bugs fixed as I have recently learned that one of our libraires is relying on using it for accessibility reasons. So I think as long as no evil hacks are involved and they go the normal way in, fixing IE bugs is a nice thing to do.
Comment 5 David Cook 2014-08-05 03:52:41 UTC
(In reply to Katrin Fischer from comment #4)
> I am happy to see IE bugs fixed as I have recently learned that one of our
> libraires is relying on using it for accessibility reasons. So I think as
> long as no evil hacks are involved and they go the normal way in, fixing IE
> bugs is a nice thing to do.

Thanks, Katrin :).

Yeah, I refuse to send in patches with IE hacks. 

In this case, that "return true" shouldn't be there regardless of the browser (although it only plays up in IE).

I have another patch for "window.open" issues where IE can't handle spaces in the "name" parameter, but Mozilla recommends not having spaces there anyway.

For other cases, feature detection works pretty well for Javascript issues.

CSS can get tricky... but CSS is also really easy to localize, so I don't worry too much about that.
Comment 6 Owen Leonard 2014-08-13 15:52:09 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2014-08-15 17:29:28 UTC
Created attachment 30857 [details] [review]
[PASSED QA] Bug 12325 - Marc and Card view in Z39.50 Results doesn't work properly in IE 8

This patch removes an unnecessary and problematic "return true" which
causes conflicts with Greybox when clicking on "MARC" or "Card" in
the search results of a Z39.50 search. The result is that the user is
taken to the showmarc.pl page and cannot return to the results, which
makes Z39.50 searching pretty much unusable for IE 8 users.

This patch also cleans up the "inline links" display for the separate
Z39.50 search pages. The 'biblio' version now has inline links for LCCN,
and the 'authority' version now prevents inline links for the MARC
and Import cells.

_TEST PLAN_

1) Open Internet Explorer 8 (if you don't have IE 8, use whatever Internet
Explorer version you do have. You can use F12 developer tools to emulate it.)
2) Go to Cataloguing > New from Z39.50
3) Do a search that will get results

(N.B. if you're using IE9+, hit F12 in the pop-up and change to IE 8 mode)

4) Click on "MARC" or "Card".
5) Note that Greybox tries to open a pop-over, but instead follows the
link to showmarc.pl. (If you're on IE 8, you can't use "Back" to go
back to the search results. If you're using an emulated version, you
can.)

(If you're testing the inline links, click on the LCCN cell for a biblio
search (not that no inline links appear), or MARC and Import cells when
doing an authority search (not that inline links do appear). Look at
bug 10096 for authority z39.50 server details.)

6) Apply the patch

7) Repeat steps 1-5
8) Note that Greybox now appears correctly when clicking "MARC" and
"Card"

9) Note also that inline links now appear for LCCN cell for biblio
searches, and inline links no longer appear for MARC and Import
cells when doing authority searches.

10) Profit

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested for regressions in Firefox and Chromium, no problems found.
Passes tests and QA script.
Comment 8 Tomás Cohen Arazi 2014-08-26 18:43:20 UTC
Patch pushed to master.

Thanks David!