Bug 17057 - Remove event attributes from holds template
Summary: Remove event attributes from holds template
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 23458
  Show dependency treegraph
 
Reported: 2016-08-05 14:51 UTC by Owen Leonard
Modified: 2019-09-05 15:01 UTC (History)
4 users (show)

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


Attachments
Bug 17057 - Remove event attributes from holds template (9.84 KB, patch)
2016-08-05 16:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17057 - Remove event attributes from holds template (9.85 KB, patch)
2016-08-06 14:27 UTC, Claire Gravely
Details | Diff | Splinter Review
[PASSED QA] Bug 17057 - Remove event attributes from holds template (9.92 KB, patch)
2016-08-07 09:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17057: Remove event attributes from holds template (2.83 KB, patch)
2019-08-13 14:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17057: (follow-up) Replace onsubmit attributes (2.38 KB, patch)
2019-08-13 16:46 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17057: Remove event attributes from holds template (2.88 KB, patch)
2019-08-16 18:29 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 17057: (follow-up) Replace onsubmit attributes (2.43 KB, patch)
2019-08-16 18:29 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 17057: Remove event attributes from holds template (2.94 KB, patch)
2019-08-17 05:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17057: (follow-up) Replace onsubmit attributes (2.49 KB, patch)
2019-08-17 05:57 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 Owen Leonard 2016-08-05 14:51:55 UTC
reserve/request.tt contains several event attributes like "onclick" which should be removed in favor of defining events in JavaScript.
Comment 1 Owen Leonard 2016-08-05 16:09:02 UTC Comment hidden (obsolete)
Comment 2 Claire Gravely 2016-08-06 14:27:19 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2016-08-07 09:48:48 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2019-08-13 14:59:23 UTC
Created attachment 92178 [details] [review]
Bug 17057: Remove event attributes from holds template

My orginal patch for this bug contained a lot of changes made obsolete
by Bug 19469, which created holds_table.inc.

This new version makes one minor change only to request.tt, adding a
class name and removing an onclick attribute.

To test, apply the patch and place an item-level hold on a title.

Start the process of placing another hold on that item. On the "place
hold" screen, in the table of items, you should see a "Cancel hold" link
for the item you previously placed on hold.

 - Click the "Cancel hold" link
   - Clicking "Cancel" in the confirmation dialog should cancel the
     operation.
   - Clicking "OK" should cancel the hold.
Comment 5 Owen Leonard 2019-08-13 16:46:27 UTC
Created attachment 92179 [details] [review]
Bug 17057: (follow-up) Replace onsubmit attributes

This patch removes the "onsubmit" attribute from the place hold form.

To test, apply the patch and test placing a hold:

Begin the process of placing a hold on a single title. On the
place hold screen, uncheck the "Hold next available item" checkbox and
click the "Place hold" button.

You should be get an alert saying "Please select an item to place a
hold." Select an item and click the "Place hold" button again. The hold
should be placed.

This patch affects the form validation for placing multiple holds, but I
don't think the JavaScript ever has a chance to run: The template blocks
the user from placing multiple holds if one of the titles has no items.
Comment 6 Maryse Simard 2019-08-16 18:29:36 UTC
Created attachment 92293 [details] [review]
Bug 17057: Remove event attributes from holds template

My orginal patch for this bug contained a lot of changes made obsolete
by Bug 19469, which created holds_table.inc.

This new version makes one minor change only to request.tt, adding a
class name and removing an onclick attribute.

To test, apply the patch and place an item-level hold on a title.

Start the process of placing another hold on that item. On the "place
hold" screen, in the table of items, you should see a "Cancel hold" link
for the item you previously placed on hold.

 - Click the "Cancel hold" link
   - Clicking "Cancel" in the confirmation dialog should cancel the
     operation.
   - Clicking "OK" should cancel the hold.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 7 Maryse Simard 2019-08-16 18:29:40 UTC
Created attachment 92294 [details] [review]
Bug 17057: (follow-up) Replace onsubmit attributes

This patch removes the "onsubmit" attribute from the place hold form.

To test, apply the patch and test placing a hold:

Begin the process of placing a hold on a single title. On the
place hold screen, uncheck the "Hold next available item" checkbox and
click the "Place hold" button.

You should be get an alert saying "Please select an item to place a
hold." Select an item and click the "Place hold" button again. The hold
should be placed.

This patch affects the form validation for placing multiple holds, but I
don't think the JavaScript ever has a chance to run: The template blocks
the user from placing multiple holds if one of the titles has no items.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 8 Katrin Fischer 2019-08-17 05:57:26 UTC
Created attachment 92329 [details] [review]
Bug 17057: Remove event attributes from holds template

My orginal patch for this bug contained a lot of changes made obsolete
by Bug 19469, which created holds_table.inc.

This new version makes one minor change only to request.tt, adding a
class name and removing an onclick attribute.

To test, apply the patch and place an item-level hold on a title.

Start the process of placing another hold on that item. On the "place
hold" screen, in the table of items, you should see a "Cancel hold" link
for the item you previously placed on hold.

 - Click the "Cancel hold" link
   - Clicking "Cancel" in the confirmation dialog should cancel the
     operation.
   - Clicking "OK" should cancel the hold.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2019-08-17 05:57:29 UTC
Created attachment 92330 [details] [review]
Bug 17057: (follow-up) Replace onsubmit attributes

This patch removes the "onsubmit" attribute from the place hold form.

To test, apply the patch and test placing a hold:

Begin the process of placing a hold on a single title. On the
place hold screen, uncheck the "Hold next available item" checkbox and
click the "Place hold" button.

You should be get an alert saying "Please select an item to place a
hold." Select an item and click the "Place hold" button again. The hold
should be placed.

This patch affects the form validation for placing multiple holds, but I
don't think the JavaScript ever has a chance to run: The template blocks
the user from placing multiple holds if one of the titles has no items.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Martin Renvoize 2019-08-19 09:25:14 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Fridolin SOMERS 2019-09-05 15:01:07 UTC
Enhancement not pushed to 19.05.x