Bug 27508 - Can't duplicate the MARC field tag with JavaScript if option "advancedMARCeditor" is set to "Don't display"
Summary: Can't duplicate the MARC field tag with JavaScript if option "advancedMARCedi...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Peter Vashchuk
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-21 12:10 UTC by Peter Vashchuk
Modified: 2021-02-11 22:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03


Attachments
Bug 27508: do nothing if tag label elements don't exist (2.56 KB, patch)
2021-01-21 12:14 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 27508: do nothing if tag label elements don't exist (2.64 KB, patch)
2021-01-21 16:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27508: do nothing if tag label elements don't exist (2.78 KB, patch)
2021-01-25 09:48 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Vashchuk 2021-01-21 12:10:50 UTC

    
Comment 1 Peter Vashchuk 2021-01-21 12:14:59 UTC
Created attachment 115535 [details] [review]
Bug 27508: do nothing if tag label elements don't exist

When JavaScript make copy of DOM set for MARC tag, it tries to reassign label elements to another id, the problem is there is no check for existence of that label element, so in case when option "advancedMARCeditor" is set to "Don't display", labels were absent which caused JavaScript to crush in the middle of process with uncaught exception.

This patch wraps this label assignment in try/catch hook like it's done in the code around for other elements, to allow the script to proceed when description labels are not present.

To test:
1) Check in the system preferences interface subsection that "advancedMARCeditor" is set to "Display".
2) Go to the "Add MARC record" cataloguing section and press "Repeat this tag" button near the MARC tag record to ensure that it makes duplicate in the interface as expected.
3) Then again in the system preferences interface subsection change "advancedMARCeditor" to "Don't display".
4) Return back to the "Add MARC record" interface, refresh the page to have no MARC field labels displayed, and try duplicating the tag again.
5) Ensure that it doesn't work, also you can notice a JavaScript error "Cannot read property "setAttribute" of undefined..." in the JavaScript console of your browser.
6) Apply the patch.
7) Repeat the test sequence (changes will be in the step 5), ensure that the tag gets duplicated even when description labels are not present.
Comment 2 Owen Leonard 2021-01-21 16:19:25 UTC
Created attachment 115574 [details] [review]
Bug 27508: do nothing if tag label elements don't exist

When JavaScript make copy of DOM set for MARC tag, it tries to reassign
label elements to another id, the problem is there is no check for
existence of that label element, so in case when option
"advancedMARCeditor" is set to "Don't display", labels were absent
which caused JavaScript to crush in the middle of process with uncaught
exception.

This patch wraps this label assignment in try/catch hook like it's done
in the code around for other elements, to allow the script to proceed
when description labels are not present.

To test:
1) Check in the system preferences interface subsection that
   "advancedMARCeditor" is set to "Display".
2) Go to the "Add MARC record" cataloguing section and press "Repeat
   this tag" button near the MARC tag record to ensure that it makes
   duplicate in the interface as expected.
3) Then again in the system preferences interface subsection change
   "advancedMARCeditor" to "Don't display".
4) Return back to the "Add MARC record" interface, refresh the page to
   have no MARC field labels displayed, and try duplicating the tag
   again.
5) Ensure that it doesn't work, also you can notice a JavaScript error
   "Cannot read property "setAttribute" of undefined..." in the
   JavaScript console of your browser.
6) Apply the patch.
7) Repeat the test sequence (changes will be in the step 5), ensure that
   the tag gets duplicated even when description labels are not present.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Joonas Kylmälä 2021-01-25 09:48:29 UTC
Created attachment 115739 [details] [review]
Bug 27508: do nothing if tag label elements don't exist

When JavaScript make copy of DOM set for MARC tag, it tries to reassign
label elements to another id, the problem is there is no check for
existence of that label element, so in case when option
"advancedMARCeditor" is set to "Don't display", labels were absent
which caused JavaScript to crush in the middle of process with uncaught
exception.

This patch wraps this label assignment in try/catch hook like it's done
in the code around for other elements, to allow the script to proceed
when description labels are not present.

To test:
1) Check in the system preferences interface subsection that
   "advancedMARCeditor" is set to "Display".
2) Go to the "Add MARC record" cataloguing section and press "Repeat
   this tag" button near the MARC tag record to ensure that it makes
   duplicate in the interface as expected.
3) Then again in the system preferences interface subsection change
   "advancedMARCeditor" to "Don't display".
4) Return back to the "Add MARC record" interface, refresh the page to
   have no MARC field labels displayed, and try duplicating the tag
   again.
5) Ensure that it doesn't work, also you can notice a JavaScript error
   "Cannot read property "setAttribute" of undefined..." in the
   JavaScript console of your browser.
6) Apply the patch.
7) Repeat the test sequence (changes will be in the step 5), ensure that
   the tag gets duplicated even when description labels are not present.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 4 Jonathan Druart 2021-01-27 13:37:50 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 5 Fridolin Somers 2021-01-29 14:48:43 UTC
Pushed to 20.11.x for 20.11.03
Comment 6 Andrew Fuerste-Henry 2021-02-04 21:41:07 UTC
Cannot recreate error on 20.05. Not backporting.