Bug 11905

Summary: when editing item, cursor is positioning in search box and not to item
Product: Koha Reporter: Marjorie Barry-Vila <marjorie.barry-vila>
Component: CatalogingAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, m.de.rooy, nick, paola.rossi, sandboxes, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14409
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: search box edit item
Patch to remove js from cataloging-search
Bug 11905 - when editing item, cursor is positioning in search box and not to item
[SIGNED-OFF] Bug 11905 - when editing item, cursor is positioning in search box and not to item
Patch to remove js from cataloging-search and add to individual tt files
Bug 11905 Patch to remove js from cataloging-search and add to individual tt files
Bug 11905 - when editing item, cursor is positioning in search box and not to item

Description Marjorie Barry-Vila 2014-03-06 19:11:09 UTC Comment hidden (obsolete)
Comment 1 Nick Clemens (kidclamp) 2014-04-12 14:54:07 UTC
I am seeing the same thing in 3.12 and 3.14.

I think the script on the page is the issue, setting the focus in search and overriding the anchor, with JS disabled the page loads as expected.

This bit seems to be it:

<script>$(document).ready(function(){
	$("input[name=q]:eq(0)").focus();
});</script>
Comment 2 Marjorie Barry-Vila 2014-04-22 18:15:09 UTC
(In reply to Nick Clemens from comment #1)
> I am seeing the same thing in 3.12 and 3.14.
> 
> I think the script on the page is the issue, setting the focus in search and
> overriding the anchor, with JS disabled the page loads as expected.
> 
> This bit seems to be it:
> 
> <script>$(document).ready(function(){
> 	$("input[name=q]:eq(0)").focus();
> });</script>

Thanks Nick for explanation.
Do you know a script js to remove focus?

Marjorie
Comment 3 Nick Clemens (kidclamp) 2014-04-23 01:32:21 UTC
(In reply to Marjorie Barry-Vila from comment #2)
> (In reply to Nick Clemens from comment #1)
> 
> Thanks Nick for explanation.
> Do you know a script js to remove focus?
> 
> Marjorie

From what I see the intranetusersjs loads before the script on the page, so you have to execute on window load to be able to steal focus.  Below should work

$(window).load(function (){
    $('[id^="tag_952_subfield_0_"]').focus();
});


I think the bigger issue though is that cataloging-search.inc is set to take focus on every page where present.

The link created when you click to edit an item has an anchor at the end and I am guessing the intention was, at one point, to set the focus to the edititem id once an item was being edited.  I do think that is the more useful fonctionality

Bug 11325 seems to be centered around the same issue with focus on the search box.
Comment 4 Nick Clemens (kidclamp) 2014-05-10 18:23:38 UTC Comment hidden (obsolete)
Comment 5 Nick Clemens (kidclamp) 2014-05-10 18:26:05 UTC
more description: 

Currently the cataloging-search.inc has js that takes focus on any page it is added to.  I think that focus should be in the individual templates for each page rather than in the search for all.

Above patch attempts to resolve this issue by removing the focus js from cataloging-search.inc
Comment 6 Biblibre Sandboxes 2014-06-04 22:10:50 UTC
Patch tested with a sandbox, by Christopher Brannon <cbrannon@cdalibrary.org>
Comment 7 Biblibre Sandboxes 2014-06-04 22:11:13 UTC Comment hidden (obsolete)
Comment 8 Juhani Seppälä 2014-06-05 05:23:54 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2014-06-06 14:08:28 UTC
The include file cataloging-search.inc is used in 3 templates. I am not sure this is the good way to do.
Maybe should you move the js code from the include file to the 2 other tt files (not sure it is useful for merge.tt).

Marked as Failed QA.
Comment 10 Nick Clemens (kidclamp) 2014-06-07 15:26:09 UTC
Agreed, I thought about adding to the others when I removed it, it doesn't hurt on merge.tt as there is no other place for focus so adding the js into both.
Comment 11 Nick Clemens (kidclamp) 2014-06-07 15:34:38 UTC Comment hidden (obsolete)
Comment 12 Paola Rossi 2014-06-13 13:22:57 UTC
I've applied against master 3.17.00.007

Everything's OK.

So I pass the patch to "Signed Off" status.
Comment 13 Paola Rossi 2014-06-13 13:27:22 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2014-06-13 13:43:08 UTC
Created attachment 28839 [details] [review]
Bug 11905 - when editing item, cursor is positioning in search box and not to item

cataloging-search.inc had embedded js to set focus into the search box.
This patch removes that js from the cataloging-search.inc and adds it into
addbooks.tt and merge.tt to set focus on those pages and allow focus to be set
to the edit items form on additem.tt

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 15 Tomás Cohen Arazi (tcohen) 2014-07-30 15:27:38 UTC
Patch pushed to master.

Thanks Nick!