Bug 16238

Summary: Upgrade jQuery in staff client: use .prop() instead of .attr()
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Architecture, internals, and plumbingAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bgkriegel, jonathan.druart, kyle, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15883    
Bug Blocks: 16394    
Attachments: Bug 16238: Use .prop() instead of .attr() for 'checked'
Bug 16238: QA fix: remove tab characters
Bug 16238: Use .prop() instead of .attr() for 'checked'
Bug 16238: QA fix: remove tab characters
Bug 16238: Use .prop() instead of .attr() for 'disabled'
Bug 16238: Use .prop() instead of .attr() for 'readonly'
Bug 16238: Use .prop() instead of .attr() for 'checked'
Bug 16238: QA fix: remove tab characters
Bug 16238: Use .prop() instead of .attr() for 'disabled'
Bug 16238: Use .prop() instead of .attr() for 'readonly'
[SIGNED-OFF] Bug 16238: Use .prop() instead of .attr() for 'checked'
[SIGNED-OFF] Bug 16238: QA fix: remove tab characters
[SIGNED-OFF] Bug 16238: Use .prop() instead of .attr() for 'disabled'
[SIGNED-OFF] Bug 16238: Use .prop() instead of .attr() for 'readonly'
Bug 16238 [Follow-up] Upgrade jQuery in staff client: use .prop() instead of .attr()
[SIGNED-OFF] Bug 16238 [Follow-up] Upgrade jQuery in staff client: use .prop() instead of .attr()
Bug 16238: Use .prop() instead of .attr() for 'checked'
Bug 16238: QA fix: remove tab characters
Bug 16238: Use .prop() instead of .attr() for 'disabled'
Bug 16238: Use .prop() instead of .attr() for 'readonly'
Bug 16238 [Follow-up] Upgrade jQuery in staff client: use .prop() instead of .attr()

Description Julian Maurice 2016-04-11 08:06:09 UTC
According to https://jquery.com/upgrade-guide/1.9/#attr-versus-prop-
.attr() is no longer correct to access the checked state of a checkbox.
Comment 1 Julian Maurice 2016-04-11 08:08:50 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2016-04-12 15:05:08 UTC
QA tools complain at 2 places:


koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc
forbidden pattern: tab char (line 6)

koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
forbidden pattern: tab char (line 169)
Comment 3 Julian Maurice 2016-04-13 08:31:30 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2016-04-13 12:38:35 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2016-04-13 12:39:33 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-04-13 13:51:02 UTC
What about the others occurrences?
I have found some others at the OPAC:
   git grep '\.attr' **/*.tt|grep "'checked'"
Comment 7 Julian Maurice 2016-04-13 14:06:19 UTC
(In reply to Jonathan Druart from comment #6)
> What about the others occurrences?
> I have found some others at the OPAC:
>    git grep '\.attr' **/*.tt|grep "'checked'"
I don't know how I could have missed so many of them. I'll provide a followup
Comment 8 Julian Maurice 2016-04-13 14:09:17 UTC
(In reply to Julian Maurice from comment #7)
> (In reply to Jonathan Druart from comment #6)
> > What about the others occurrences?
> > I have found some others at the OPAC:
> >    git grep '\.attr' **/*.tt|grep "'checked'"
> I don't know how I could have missed so many of them. I'll provide a followup
In fact, it's completely normal, bug 15883 only upgrades jQuery for staff interface, so no changes required in OPAC
Comment 9 Jonathan Druart 2016-04-13 14:12:05 UTC
What do you only fix checked?
There are also disabled and readonly.
Comment 10 Jonathan Druart 2016-04-13 14:12:24 UTC
s/What/Why
Comment 11 Julian Maurice 2016-04-13 14:24:35 UTC
(In reply to Jonathan Druart from comment #9)
> Why do you only fix checked?
> There are also disabled and readonly.

No particular reason. I suppose this should be done as well
Comment 12 Julian Maurice 2016-04-13 18:29:08 UTC Comment hidden (obsolete)
Comment 13 Julian Maurice 2016-04-13 18:51:49 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2016-04-21 14:01:31 UTC
Sorry, does not apply:

Applying: Bug 16238: Use .prop() instead of .attr() for 'checked'
Applying: Bug 16238: QA fix: remove tab characters
Applying: Bug 16238: Use .prop() instead of .attr() for 'disabled'
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt
Comment 15 Owen Leonard 2016-04-21 18:24:03 UTC
That merge problem isn't very serious. I was able to correct it while testing, but I'm still going through all the changes before I can sign off.
Comment 16 Julian Maurice 2016-04-22 07:15:31 UTC Comment hidden (obsolete)
Comment 17 Julian Maurice 2016-04-22 07:15:33 UTC Comment hidden (obsolete)
Comment 18 Julian Maurice 2016-04-22 07:15:36 UTC Comment hidden (obsolete)
Comment 19 Julian Maurice 2016-04-22 07:15:39 UTC Comment hidden (obsolete)
Comment 20 Julian Maurice 2016-04-22 07:16:05 UTC
Patches rebased on master
Comment 21 Owen Leonard 2016-04-22 12:57:12 UTC Comment hidden (obsolete)
Comment 22 Owen Leonard 2016-04-22 12:57:19 UTC Comment hidden (obsolete)
Comment 23 Owen Leonard 2016-04-22 12:57:25 UTC Comment hidden (obsolete)
Comment 24 Owen Leonard 2016-04-22 12:57:32 UTC Comment hidden (obsolete)
Comment 25 Owen Leonard 2016-04-22 13:03:21 UTC Comment hidden (obsolete)
Comment 26 Bernardo Gonzalez Kriegel 2016-04-26 17:01:26 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2016-04-26 17:52:32 UTC
Created attachment 50737 [details] [review]
Bug 16238: Use .prop() instead of .attr() for 'checked'

According to https://jquery.com/upgrade-guide/1.9/#attr-versus-prop-
.attr() is no longer correct to access the checked state of a checkbox.

This patch do the following replacements:
  .attr('checked')               =>  .prop('checked')
  .attr('checked, '')            =>  .prop('checked', false)
  .attr('checked, 'checked')     =>  .prop('checked', true)
  .attr('checked', boolValue)    =>  .prop('checked', boolValue)
  .removeAttr('checked')         =>  .prop('checked', false)
  .attr('checked') == 'checked'  =>  .is(':checked')

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Jonathan Druart 2016-04-26 17:52:36 UTC
Created attachment 50738 [details] [review]
Bug 16238: QA fix: remove tab characters

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 29 Jonathan Druart 2016-04-26 17:52:40 UTC
Created attachment 50739 [details] [review]
Bug 16238: Use .prop() instead of .attr() for 'disabled'

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Jonathan Druart 2016-04-26 17:52:44 UTC
Created attachment 50740 [details] [review]
Bug 16238: Use .prop() instead of .attr() for 'readonly'

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 31 Jonathan Druart 2016-04-26 17:52:48 UTC
Created attachment 50741 [details] [review]
Bug 16238 [Follow-up] Upgrade jQuery in staff client: use .prop() instead of .attr()

This patch corrects some tabs which should have been replaced with
spaces.

To test, diff against the previous commit with the '-w' flag. There
should be no visible changes.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Space changes, no errors.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 32 Kyle M Hall 2016-04-29 14:32:00 UTC
Pushed to master for the 16.05 release. Thanks Julian!