Bug 21476 - Incorrect filter prevents HTML5 media from playing in the OPAC
Summary: Incorrect filter prevents HTML5 media from playing in the OPAC
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords: rel_18_11_candidate
Depends on: 8377 13618
Blocks: 20783
  Show dependency treegraph
 
Reported: 2018-10-02 16:03 UTC by Owen Leonard
Modified: 2018-11-28 13:52 UTC (History)
7 users (show)

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


Attachments
Bug 21476: Incorrect filter prevents HTML5 media from playing in the OPAC (1.98 KB, patch)
2018-10-19 12:39 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21476: [alternate] Incorrect filter prevents HTML5 media from playing in the OPAC (2.03 KB, patch)
2018-10-23 12:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21476: [alternate] Incorrect filter prevents HTML5 media from playing in the OPAC (2.15 KB, patch)
2018-10-24 05:14 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21476: Fix YouTube propgation (1.48 KB, patch)
2018-10-31 12:23 UTC, Nick Clemens
Details | Diff | Splinter Review
Sample marc record (822 bytes, text/plain)
2018-10-31 12:24 UTC, Nick Clemens
Details
Bug 21476: Fix YouTube propgation (1.53 KB, patch)
2018-10-31 12:40 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 21476: Fix HTML5 media from playing in the OPAC - incorrect filters (2.21 KB, patch)
2018-10-31 20:25 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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