Bug 13943

*** Bug 14032 has been marked as a duplicate of this bug. ***

I've definitely experienced this as well, although in my case the problem was with Chrome and IE while it worked in Firefox.

After I reported the bug and gave up on it, I tried again in Chrome and IE... and found that they were working as expected! No buggy behaviour!

So I don't know... my boss suggested that we make the confirm_deletion function more simple or remove it all together in favour of the following inline code:

onclick="return confirm('Are you sure you want to delete this item?')"

or

onclick="return confirm(_('Are you sure you want to delete this item?'))"

Perhaps that's a thought... when I tested the first option, it worked fine.

(In reply to David Cook from comment #2)

> So I don't know... my boss suggested that we make the confirm_deletion
> function more simple or remove it all together in favour of the following
> inline code:
> 
> onclick="return confirm('Are you sure you want to delete this item?')"
> 
> or
> 
> onclick="return confirm(_('Are you sure you want to delete this item?'))"
> 
> Perhaps that's a thought... when I tested the first option, it worked fine.

Be careful :
1/ JavaScript in HTML attributes is not translatable
2/ Always use double quotes inside _() function because translation may add a single quote

(In reply to Fridolin SOMERS from comment #3)
> Be careful :
> 1/ JavaScript in HTML attributes is not translatable
> 2/ Always use double quotes inside _() function because translation may add
> a single quote

Good to know :). I don't have any plans to work on this anyway, but good to know :).

So Liz Rea is a genius! She pointed out that the problem only arises if the screen is using the result browser!

For instance:

http://192.168.1.61:2112/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1&searchid=scs_1429666452994

Deletes item after "Cancel".

http://192.168.1.61:2112/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1

Doesn't delete item after "Cancel".

--

The cause is in "koha-tmpl/intranet-tmpl/js/browser.js":

$('a[href*="biblionumber="]').click(function (ev) {
                        ev.preventDefault();
                        window.location = $(this).attr('href') + '&searchid=' + me.searchid;
                    });

Basically, this is rewriting the URL for all the other links on the page, so that when you click... "Edit", "Delete", "Normal", etc. you still have that searchid parameter.

Now... it seems to me that the thing to do is either be more sophisticated with the Javascript magic (i.e. parse the href, find the query string, add the key=value appropriately, then set the href for the element before it browser acts on it), or... handle it server-side.

Follow-up... that bit of Javascript will do a window.location regardless of the outcome of the "confirm_deletion(); return false;" onclick stuff. :(

(In reply to David Cook from comment #5)
> Basically, this is rewriting the URL 

Well, rewriting the URL and redirecting to it...

This is growing into a bigger problem - we also got a report of the same phenomenon in acquisitions now when cancelling an order.

(In reply to Katrin Fischer from comment #8)
> This is growing into a bigger problem - we also got a report of the same
> phenomenon in acquisitions now when cancelling an order.

Interesting... does the result browser Javascript get called there? That's what's causing the problem elsewhere.

Can we get a step-by-step plan to reproduce the issue please?
Does it happen with the "en" templates?
What versions of chromium, all?
Is there a JS error?

Hi Joubu,

try the following:
- Do a search with mulitple results
- Go to the detail page (make sure results browser shows up!)
- Use the "Edit items" link from the toolbar
- Delete item
- Choose "cancel"
- Watch item getting deleted

I just tested this in Firefox - hope it's going to happen consequently now at least...

Jonathan, have you tested this? 

At a glance, it looks like the main delete button will work because the href attribute points to "/cgi-bin/koha/mainpage.pl" instead of "/cgi-bin/koha/cataloguing/additem.pl?op=delitem&biblionumber=[% biblionumber %]&itemnumber=[% item_loo.itemnumber %]", but I don't know why you'd point at "/cgi-bin/koha/mainpage.pl"... I think that would be a surprising result when successfully deleting an item, no?

I imagine the inline delete link will work since browser.js should try to bind the problematic click handler using the selector $('a[href*="biblionumber="]') before the inline links get created. 

Anyway... just some thoughts...

(In reply to David Cook from comment #13)
> Jonathan, have you tested this? 
> 
> At a glance, it looks like the main delete button will work because the href
> attribute points to "/cgi-bin/koha/mainpage.pl" instead of
> "/cgi-bin/koha/cataloguing/additem.pl?op=delitem&biblionumber=[%
> biblionumber %]&itemnumber=[% item_loo.itemnumber %]", but I don't know
> why you'd point at "/cgi-bin/koha/mainpage.pl"... I think that would be a
> surprising result when successfully deleting an item, no?

Sorry, it was a stupid test.
I was happy, it works (the deletion did not work when I canceled the deletion).
So I tested, but not completely.

> I imagine the inline delete link will work since browser.js should try to
> bind the problematic click handler using the selector
> $('a[href*="biblionumber="]') before the inline links get created. 
> 
> Anyway... just some thoughts...

Thanks for that, I did not catch that yesterday! It was very useful.
The last patch should work as expected.

Created attachment 42184 [details] [review]
[PASSED QA] Bug 13943: Prevent the deletion of items if the user cancels

On the edit items page, there is some weird JS code: if the user clicks
on the delete link and then cancel, the item is deleted anyway.

It's caused by the following JS code in browser.js
  $('a[href*="biblionumber="]').click(function (ev) {
      ev.preventDefault();
      window.location = $(this).attr('href') + '&searchid=' + me.searchid;
  });

Test plan:
- Do a search with multiple results
- Go to the detail page (make sure results browser shows up!)
- Use the "Edit items" link from the toolbar
- Delete an item (try both way)
- Choose "cancel"
- Delete an item (try both way) and confirm the deletion
You should see the browser after the deletion and the item should have
been deleted correctly.
- Edit an item (try both way)
You should see the browser (did not work before this patch)

Note: Before this patch, the 2 first columns didn't contain the
edit/delete item links, now it's only the 1st one.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Much better!

Patch pushed to master.

Thanks Jonathan!

pushed to 3.20.x will be in 3.20.4

Pushed to 3.18.x will be in 3.18.11