Bug 21243

Summary: Regression: SRU mapping popup for bibliographic records is unstyled
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 21151    
Bug Blocks: 20743    
Attachments: Bug 21243: Regression: SRU mapping popup for bibliographic records is unstyled
Bug 21243: Regression: SRU mapping popup for bibliographic records is unstyled
Bug 21243: Regression: SRU mapping popup for bibliographic records is unstyled
Bug 21243: (follow-up) Remove extra '</div>' tag

Description Owen Leonard 2018-08-17 14:18:08 UTC
Bug 21151 removed [% INCLUDE 'doc-head-close.inc' %] from sru_modmapping.tt. It means the page loads without any CSS at all. I don't think this was intentional?
Comment 1 Owen Leonard 2018-08-17 14:27:19 UTC
Created attachment 77982 [details] [review]
Bug 21243: Regression: SRU mapping popup for bibliographic records is unstyled

This patch corrects a regression caused by Bug 21151: The include file
which adds CSS to the page was removed from the template.

To test, apply the patch and go to Administration -> Z39.50/SRU servers
-> New SRU server.

- Click the "Modify" button next to "SRU Search fields mapping"
- Verify that the pop-up window is correctly styled.
Comment 2 Katrin Fischer 2018-08-17 16:28:02 UTC
Hm, my patch on bug 21552 didn't remove it, only added a parameter - wonder what happened there? 

+[% INCLUDE 'doc-head-close.inc' popup => 1 %]
Comment 3 Claire Gravely 2018-08-21 07:21:28 UTC
Created attachment 78026 [details] [review]
Bug 21243: Regression: SRU mapping popup for bibliographic records is unstyled

This patch corrects a regression caused by Bug 21151: The include file
which adds CSS to the page was removed from the template.

To test, apply the patch and go to Administration -> Z39.50/SRU servers
-> New SRU server.

- Click the "Modify" button next to "SRU Search fields mapping"
- Verify that the pop-up window is correctly styled.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 4 Kyle M Hall 2018-08-21 12:01:04 UTC
Created attachment 78041 [details] [review]
Bug 21243: Regression: SRU mapping popup for bibliographic records is unstyled

This patch corrects a regression caused by Bug 21151: The include file
which adds CSS to the page was removed from the template.

To test, apply the patch and go to Administration -> Z39.50/SRU servers
-> New SRU server.

- Click the "Modify" button next to "SRU Search fields mapping"
- Verify that the pop-up window is correctly styled.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Katrin Fischer 2018-08-21 12:02:37 UTC
Is reverting bug 21151 and repushing it correctly also an option? It adds a missing div as well.
Comment 6 Nick Clemens 2018-08-21 13:17:19 UTC
(In reply to Katrin Fischer from comment #5)
> Is reverting bug 21151 and repushing it correctly also an option? It adds a
> missing div as well.

The div does seem to have been added, it was a typo during rebase that I deleted the line, I think we can just add it here

bug 13618 would make revert more difficult I think
Comment 7 Nick Clemens 2018-08-21 13:35:04 UTC
(In reply to Nick Clemens from comment #6)
> (In reply to Katrin Fischer from comment #5)
> > Is reverting bug 21151 and repushing it correctly also an option? It adds a
> > missing div as well.
> 
> The div does seem to have been added, it was a typo during rebase that I
> deleted the line, I think we can just add it here
> 
> bug 13618 would make revert more difficult I think

Looking again, I think that extra closing div is incorrect. We have a closing div in intranet-bottom.inc - with 21151 we have an extra closing tag (run the generated html through the wc3 validator)
Comment 8 Nick Clemens 2018-08-21 13:43:22 UTC
Created attachment 78044 [details] [review]
Bug 21243: (follow-up) Remove extra '</div>' tag

To test:
1 - Go to Admin->Z39/SRU servers
2 - Pick a server or add one
3 - Click 'Modify' button for mappings
4 - Copy the HTML of the popup into w3c validator
https://validator.w3.org/#validate_by_input
5 - Note extra closing div
6 - Apply patch
7 - Repeat
8 - Success
Comment 9 Katrin Fischer 2018-08-21 13:47:19 UTC
Ok, probably also fixed by adding SRU for authorities then. I had checked the validator at the time, but the other patches altered page structure.
Comment 10 Nick Clemens 2018-08-21 13:59:02 UTC
Awesome work all!

Pushed to master for 18.11
Comment 11 Martin Renvoize 2018-08-24 10:27:00 UTC
Pushed to 18.05.x for 18.05.03
Comment 12 Fridolin Somers 2018-09-21 10:11:30 UTC
Depends on Bug 21151 not in 17.11.x