Bug 21476

Summary: Incorrect filter prevents HTML5 media from playing in the OPAC
Product: Koha Reporter: Owen Leonard <oleonard>
Component: OPACAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: alexbuckley, fridolin.somers, jonathan.druart, josef.moravec, katrin.fischer, kohadevinim, martin.renvoize
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21742
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 8377, 13618    
Bug Blocks: 20783    
Attachments: Bug 21476: Incorrect filter prevents HTML5 media from playing in the OPAC
Bug 21476: [alternate] Incorrect filter prevents HTML5 media from playing in the OPAC
Bug 21476: [alternate] Incorrect filter prevents HTML5 media from playing in the OPAC
Bug 21476: Fix YouTube propgation
Sample marc record
Bug 21476: Fix YouTube propgation
Bug 21476: Fix HTML5 media from playing in the OPAC - incorrect filters

Description Owen Leonard 2018-10-02 16:03:20 UTC
There is an extra set of quotes in the markup for HTML5 media embeds in the OPAC which prevents media from playing.
Comment 1 Owen Leonard 2018-10-19 12:39:04 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2018-10-19 14:27:15 UTC
I did not check what was going on exactly but the code at the intranet is using the html filter. I think we should use the exact same code at the OPAC and the intranet sides.
Comment 3 Nick Clemens 2018-10-23 12:31:15 UTC Comment hidden (obsolete)
Comment 4 Alex Buckley 2018-10-24 05:14:33 UTC
Created attachment 81047 [details] [review]
Bug 21476: [alternate] Incorrect filter prevents HTML5 media from playing in the OPAC

This patch makes a correction to a template filter to prevent
incorrectly-encoded HTML in media source links which were preventing
media from embedding correctly in the OPAC.

To test, apply the patch and enable the HTML5MediaEnabled and
HTML5MediaYouTube system preferences.

- Edit a record to include links to videos in 856$u. For example:
  - https://www.youtube.com/watch?v=grTwH6Evdfc
    - https://archive.org/download/POPEYEMeetsSindbadTheSailor1936/POPEYE%20meets%20Sindbad%20The%20Sailor%20%281936%29.ogv

    View the record in the OPAC and confirm that the videos display in
    the
    "Play media" tab and that they play correctly.

Tested and works as described, also matches the intranet code.

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 5 Alex Buckley 2018-10-24 05:16:25 UTC
I tested both patches and they both work nicely. But if we are aiming to make the OPAC code the same as the intranet code (as Jonathan recommended in comment #2) then the second patch does that.
Comment 6 Devinim 2018-10-24 10:44:17 UTC
Tested and works as described. But, there is a MIMETYPE error when we try to apply that scenario.
we may have two 856 fields . If  the first field is url and second field is media it works without error. However, if the first field is media and second field isn't , it gives an error.
Comment 7 Katrin Fischer 2018-10-25 21:30:00 UTC
setting to failed qa to get the attention of the developer - see comment before this!
Comment 8 Nick Clemens 2018-10-30 19:22:29 UTC
(In reply to Devinim from comment #6)
> Tested and works as described. But, there is a MIMETYPE error when we try to
> apply that scenario.
> we may have two 856 fields . If  the first field is url and second field is
> media it works without error. However, if the first field is media and
> second field isn't , it gives an error.

I cannot recreate this error, can you supply a sample record, or the links used?
Comment 9 Devinim 2018-10-31 11:23:36 UTC
(In reply to Nick Clemens from comment #8)
> (In reply to Devinim from comment #6)
> > Tested and works as described. But, there is a MIMETYPE error when we try to
> > apply that scenario.
> > we may have two 856 fields . If  the first field is url and second field is
> > media it works without error. However, if the first field is media and
> > second field isn't , it gives an error.
> 
> I cannot recreate this error, can you supply a sample record, or the links
> used?

You can look at the example by clicking on the link below.

- http://staff-bug21476.sb1.sandboxes.bywatersolutions.com/cgi-bin/koha/catalogue/detail.pl?biblionumber=4
username/password => koha/koha

If you play the second media you will see MIMETYPE error.
Comment 10 Nick Clemens 2018-10-31 12:23:50 UTC
Created attachment 81713 [details] [review]
Bug 21476: Fix YouTube propgation

The loop to find all media has an $isyoutube variable. This was declared
outside the loop and never reset. Once a YouTube video was encountered
all subsequent videos were considered as YouTube. This patch reduces the
scope of the variable and resets per loop.

To test:
1 - Enable HTML5Media and HTML5MediaYouTube
2 - Add 856$u to record for a youtube vido and a pdf (sample record
        attached to bug)
3 - View the record, the Play media tab has two video boxes
4 - Play the second, it fails
5 - Apply patch
6 - View record, Play media should include only one video link
Comment 11 Nick Clemens 2018-10-31 12:24:26 UTC
Created attachment 81714 [details]
Sample marc record
Comment 12 ByWater Sandboxes 2018-10-31 12:40:03 UTC
Created attachment 81721 [details] [review]
Bug 21476: Fix YouTube propgation

The loop to find all media has an $isyoutube variable. This was declared
outside the loop and never reset. Once a YouTube video was encountered
all subsequent videos were considered as YouTube. This patch reduces the
scope of the variable and resets per loop.

To test:
1 - Enable HTML5Media and HTML5MediaYouTube
2 - Add 856$u to record for a youtube vido and a pdf (sample record
        attached to bug)
3 - View the record, the Play media tab has two video boxes
4 - Play the second, it fails
5 - Apply patch
6 - View record, Play media should include only one video link

Signed-off-by: Devinim <kohadevinim@devinim.com.tr>
Comment 13 Jonathan Druart 2018-10-31 13:28:38 UTC
(In reply to ByWater Sandboxes from comment #12)
> Created attachment 81721 [details] [review] [review]
> Bug 21476: Fix YouTube propgation

At first glance I'd say this patch should be moved to its own bug report.
It seems to be caused by bug 17231, which is in stables.
Comment 14 Jonathan Druart 2018-10-31 20:20:29 UTC
(In reply to Jonathan Druart from comment #13)
> (In reply to ByWater Sandboxes from comment #12)
> > Created attachment 81721 [details] [review] [review] [review]
> > Bug 21476: Fix YouTube propgation
> 
> At first glance I'd say this patch should be moved to its own bug report.
> It seems to be caused by bug 17231, which is in stables.

Will be moved to bug 21742
Comment 15 Jonathan Druart 2018-10-31 20:25:48 UTC
Created attachment 81780 [details] [review]
Bug 21476: Fix HTML5 media from playing in the OPAC - incorrect filters

This patch makes a correction to a template filter to prevent
incorrectly-encoded HTML in media source links which were preventing
media from embedding correctly in the OPAC.

To test, apply the patch and enable the HTML5MediaEnabled and
HTML5MediaYouTube system preferences.

- Edit a record to include links to videos in 856$u. For example:
  - https://www.youtube.com/watch?v=grTwH6Evdfc
    - https://archive.org/download/POPEYEMeetsSindbadTheSailor1936/POPEYE%20meets%20Sindbad%20The%20Sailor%20%281936%29.ogv

    View the record in the OPAC and confirm that the videos display in
    the
    "Play media" tab and that they play correctly.

Tested and works as described, also matches the intranet code.

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Nick Clemens 2018-11-01 19:09:21 UTC
Awesome work all!

Pushed to master for 18.11
Comment 17 Martin Renvoize 2018-11-09 16:14:26 UTC
Pushed to 18.05.x for 18.05.06.
Comment 18 Fridolin Somers 2018-11-26 06:39:22 UTC
Pushed to 17.11.x for 17.11.12