Clicking "Log in to your account" throws a fatal Javascript error, because the link has a href of "/cgi-bin/koha/opac-user.pl" and Bootstrap passes the href to Jquery as if it were a CSS ID selector. In the past, Jquery didn't throw a fatal error for this mistake, so it was fine. But now Jquery appears to be stricter with the upgrade to 3.4.1. This error causes Javascript to fail and could lead to unexpected behaviour. (I have an idea for a patch that I'll be trying out very soon.)
My first idea is to replace "/cgi-bin/koha/opac-user.pl" with #loginModal (as per https://www.w3schools.com/Bootstrap/bootstrap_ref_js_modal.asp), and that works (after disabling our Koha click handler for showing/hiding the modal). But... now I'm wondering if we had the "href" as "/cgi-bin/koha/opac-user.pl" for a reason. Maybe so the link would still work if Javascript wasn't working? So I'm going to work on an alternative idea suggested by Jonathan too...
(In reply to David Cook from comment #1) > So I'm going to work on an alternative idea suggested by Jonathan too... Oh except the alternative idea won't retain a usable link either... well I might provide it anyway, and let the community decide which they prefer.
Created attachment 100156 [details] [review] Bug 24803: Remove JS error by following Bootstrap instructions This patch patches #lodinModal to Bootstrap's click.modal.data-api event handler for the '[data-toggle="modal"] selector, so Bootstrap can handle the hide/show behaviour of the login modal. Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console
Created attachment 100157 [details] [review] Bug 24803: [Alternate patch] bypass Bootstrap click handler by using "#" This patch bypasses Bootstrap's click.modal.data-api event handler for the '[data-toggle="modal"] selector by using a "#" as the "href" Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console
Created attachment 100159 [details] [review] Bug 24803: Remove JS error by following Bootstrap instructions This patch patches #lodinModal to Bootstrap's click.modal.data-api event handler for the '[data-toggle="modal"] selector, so Bootstrap can handle the hide/show behaviour of the login modal. Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> JS error verified, and corrected. No qa errors.
Created attachment 100160 [details] [review] Bug 24803: [Alternate patch] bypass Bootstrap click handler by using "#" This patch bypasses Bootstrap's click.modal.data-api event handler for the '[data-toggle="modal"] selector by using a "#" as the "href" Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> JS error verified, and corrected. No qa errors.
(In reply to David Cook from comment #2) > (In reply to David Cook from comment #1) > > So I'm going to work on an alternative idea suggested by Jonathan too... > > Oh except the alternative idea won't retain a usable link either... well I > might provide it anyway, and let the community decide which they prefer. Both works :)
Thanks, Bernardo! I'm curious to see what QA and RM think about the best approach.
Created attachment 100191 [details] [review] Bug 24803: [Alternate] Clicking "Log in to your account" throws fatal Javascript error This patch removes the "data-toggle" attribute from the login link in order to avoid a JavaScript error. Since we have a custom click handler to trigger the modal we don't need Bootstrap's "automatic" modal handling. Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console
Created attachment 100208 [details] [review] Bug 24803: [Alternate] Clicking "Log in to your account" throws fatal Javascript error This patch removes the "data-toggle" attribute from the login link in order to avoid a JavaScript error. Since we have a custom click handler to trigger the modal we don't need Bootstrap's "automatic" modal handling. Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console Signed-off-by: David Cook <dcook@prosentient.com.au>
Nice one, Owen. I think I prefer Owen's patch best, since it keeps a usable link. But will leave it up to QA/RM. (Also, as Jonathan noted on koha-devel, GoogleOpenIDConnect will be affected by the same issue, so whatever approach we choose here should be applied there as well.)
Created attachment 100209 [details] [review] Bug 24803: [PASSED_QA] Clicking "Log in to your account" throws fatal Javascript error This patch removes the "data-toggle" attribute from the login link in order to avoid a JavaScript error. Since we have a custom click handler to trigger the modal we don't need Bootstrap's "automatic" modal handling. Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
RM: I prefer Owen's patch too, marked as PASSED_QA. If you agree, obsolete the other two.
Created attachment 100217 [details] [review] Bug 24803: Remove JS error when clicking "Log in to your account" This patch removes the "data-toggle" attribute from the login link in order to avoid a JavaScript error. Since we have a custom click handler to trigger the modal we don't need Bootstrap's "automatic" modal handling. Test plan: 1. Apply patch 2. Open F12 dev tools 3. Click "Log in to your account" on OPAC 4. Observe no Javascript errors in console Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Marcel de Rooy from comment #13) > RM: I prefer Owen's patch too, marked as PASSED_QA. If you agree, obsolete > the other two. I am not RM, but everybody prefers Owen's patch ;) I planned to QA those patches this morning, done!
Nice work everyone! Pushed to master for 20.05
Backported to 19.11.x for 19.11.05
cannot recreate in 19.05.x, no backport