Bug 12320 - $.cookie('foo', null) deprecated
Summary: $.cookie('foo', null) deprecated
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 12777 (view as bug list)
Depends on:
Blocks: 12777
  Show dependency treegraph
 
Reported: 2014-05-27 21:35 UTC by Galen Charlton
Modified: 2016-12-05 21:23 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 12320: Remove deprecated construct to delete cookie holdfor (3.10 KB, patch)
2015-04-28 10:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12320: Remove deprecated construct to delete cookie showColumns (2.90 KB, patch)
2015-04-28 10:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12320: Remove deprecated construct to delete cookie holdfor (3.18 KB, patch)
2015-05-21 17:49 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12320: Remove deprecated construct to delete cookie showColumns (3.01 KB, patch)
2015-05-21 17:49 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 12320: Remove deprecated construct to delete cookie holdfor (3.24 KB, patch)
2015-05-22 14:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 12320: Remove deprecated construct to delete cookie showColumns (3.07 KB, patch)
2015-05-22 14:57 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2014-05-27 21:35:02 UTC
With the upgrade of jquery-cookie to the latest version (see bug 11369), the following construct to delete a cookie is no longer supported:

$.cookie('foo', null)

Instead, per https://github.com/carhartl/jquery-cookie/blob/master/CHANGELOG.md, it should now be

$.removeCookie('foo');

There is at least one direct use of the deprecated construct:

koha-tmpl/intranet-tmpl/prog/en/js/pages/batchMod.js:    $.cookie("showColumns",null);

and possibly others:

koha-tmpl/intranet-tmpl/prog/en/js/staff-global.js:     $.cookie("holdfor",null, { path: "/", expires: 0 });
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:    $.cookie("holdfor",null, { path: "/", expires: 0 });
koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt:[% IF ( borrowernumber ) %]if($.cookie("holdfor") != [% borrowernumber %]){ $.cookie("holdfor",null, { path: "/", expires: 0 }); }[% ELSE %]$.cookie("holdfor",null, { path: "/", expires: 0 });[% END %]
Comment 1 Jonathan Druart 2015-04-28 10:58:31 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-04-28 10:58:35 UTC Comment hidden (obsolete)
Comment 3 Bernardo Gonzalez Kriegel 2015-05-21 17:49:15 UTC Comment hidden (obsolete)
Comment 4 Bernardo Gonzalez Kriegel 2015-05-21 17:49:22 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2015-05-22 14:57:53 UTC
Created attachment 39424 [details] [review]
[PASSED QA] Bug 12320: Remove deprecated construct to delete cookie holdfor

$.cookie('foo', null);
is deprecated and should be replaced with
  $.removeCookie('foo');

This patch replaces the occurrences for the "holdfor" cookie.

Test plan:
1/ Search for a patron
2/ On the patron detail page, click on "search to hold"
3/ Search for records
4/ On the results page, click on "Place hold" > "Forget PATRON"
5/ Reload the page.
6/ The "Place hold" button should not contain the patron anymore

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works as described, no errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2015-05-22 14:57:59 UTC
Created attachment 39425 [details] [review]
[PASSED QA] Bug 12320: Remove deprecated construct to delete cookie showColumns

  $.cookie('foo', null);
is deprecated and should be replaced with
  $.removeCookie('foo');

This patch replaces the occurrences for the "showColumns" cookie.

Before this patch, there was a bug on the batchmod tools.
To reproduce the issue:
1/ Go on the Batch item modification tool
2/ Fill the textarea with barcodes and submit
3/ Click on some column names (to create the cookie)
4/ Click on 'Show all columns" (to set the cookie to null)
5/ Don't submit and repeat steps 1 & 2
6/ You should see a js error:
  Error: Syntax error, unrecognized expression: :nth-child
    ...break;q=a}return s},m.error=function(a){throw new Error("Syntax error, unrecogni...

Test plan:
Confirm the issue has gone away and there is no regression on the column
selection

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No js error, no regressions, no errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Bernardo Gonzalez Kriegel 2015-05-30 13:46:06 UTC
*** Bug 12777 has been marked as a duplicate of this bug. ***
Comment 8 Tomás Cohen Arazi 2015-06-04 13:18:29 UTC
Patches pushed to master.

Thanks Jonathan!
Comment 9 Chris Cormack 2015-06-07 07:56:44 UTC
Pushed to 3.20.x will be in 3.20.1
Comment 10 Liz Rea 2015-06-08 04:49:55 UTC
Hi, 

This patch does not apply cleanly to 3.18.x, if you feel it should be included, please rebase for 3.18.x.

Cheers,
Liz