|
Description
Owen Leonard
2018-10-02 16:03:20 UTC
Created attachment 80932 [details] [review] Bug 21476: 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. 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. Created attachment 81013 [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. 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> 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. 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. setting to failed qa to get the attention of the developer - see comment before this! (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? (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. 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 Created attachment 81714 [details]
Sample marc record
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> (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. (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 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> Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.06. Pushed to 17.11.x for 17.11.12 |