[Bug 17231] New: HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Bug ID: 17231 Summary: HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org To recreate: With HTML5MediaYouTube enabled and WWW::YouTube::Download installed add an 856 to a record like: https://www.youtube.com/watch?v=tu0qtEwb9gE and not the media player works then use a link like: https://youtu.be/tu0qtEwb9gE And note there is no media player -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 55076 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55076&action=edit Bug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links To test: Enable HTML5MediaYouTube and ensure WWW::YouTube::Download is installed Add an 856$u to a record like: https://www.youtube.com/watch?v=tu0qtEwb9gE Verify you can see the embedded player Use the youtube shortened like (from the share button) like: https://youtu.be/tu0qtEwb9gE Verify you can see the embedded player -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Assignee|gmcharlt@gmail.com |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |claire_gravely@hotmail.com --- Comment #2 from Claire Gravely <claire_gravely@hotmail.com> --- After applying the patch: I get the following error when doing a catalogue search on the intranet for a record which already has a link in 856 $u GET http://www.youtube.com/watch?v=http://bulbapedia.bulbagarden.net/wiki/Jiggly...) failed. status: 404 Not Found at /home/claire/koha/C4/HTML5Media.pm line 106. Search for a record without an 856 $u works fine. Searching for a record with a youtube.com or youtu.be link works fine. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #3 from Claire Gravely <claire_gravely@hotmail.com> --- (In reply to Claire Gravely from comment #2)
After applying the patch: I get the following error when doing a catalogue search on the intranet for a record which already has a link in 856 $u
GET http://www.youtube.com/watch?v=http://bulbapedia.bulbagarden.net/wiki/ Jigglypuff_(Pok%C3%A9mon) failed. status: 404 Not Found at /home/claire/koha/C4/HTML5Media.pm line 106.
Search for a record without an 856 $u works fine. Searching for a record with a youtube.com or youtu.be link works fine.
Also applies to OPAC if HTML5MediaEnabled is set to OPAC and staff client. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55076|0 |1 is obsolete| | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 55091 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55091&action=edit Bug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links To test: Enable HTML5MediaYouTube and ensure WWW::YouTube::Download is installed Add an 856$u to a record like: https://www.youtube.com/watch?v=tu0qtEwb9gE Verify you can see the embedded player Use the youtube shortened like (from the share button) like: https://youtu.be/tu0qtEwb9gE Verify you can see the embedded player -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Fixed! That was bad grepping on my part, worked cause it matched everything :-) I added an eval to catch those errors as well, say if you link to something like: http://www.nytimes.com/topic/company/youtube Which will match, but is not a youtube video (In reply to Claire Gravely from comment #3)
(In reply to Claire Gravely from comment #2)
After applying the patch: I get the following error when doing a catalogue search on the intranet for a record which already has a link in 856 $u Also applies to OPAC if HTML5MediaEnabled is set to OPAC and staff client.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #6 from Claire Gravely <claire_gravely@hotmail.com> --- (In reply to Nick Clemens from comment #5)
Fixed! That was bad grepping on my part, worked cause it matched everything :-)
I added an eval to catch those errors as well, say if you link to something like: http://www.nytimes.com/topic/company/youtube
Which will match, but is not a youtube video
(In reply to Claire Gravely from comment #3)
(In reply to Claire Gravely from comment #2)
After applying the patch: I get the following error when doing a catalogue search on the intranet for a record which already has a link in 856 $u Also applies to OPAC if HTML5MediaEnabled is set to OPAC and staff client.
It now searches ok :), but if the record has two 856 $u (one for a link and one for a video for example) it does not embed the youtube video. I should note, that adding the two kinds of youtube style link does work fine on items without additional 856$u fields. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Claire Gravely from comment #6)
It now searches ok :), but if the record has two 856 $u (one for a link and one for a video for example) it does not embed the youtube video.
Hmm...It works if you put two separate 856 fields, each with their own $u In master a link plus a youtube $u in a single 856 causes an error In master as well, two regular links in a single 856 only seems to display the first link anyways. I think this is not a bug but intended functionality -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #8 from Claire Gravely <claire_gravely@hotmail.com> --- (In reply to Nick Clemens from comment #7)
(In reply to Claire Gravely from comment #6)
It now searches ok :), but if the record has two 856 $u (one for a link and one for a video for example) it does not embed the youtube video.
Hmm...It works if you put two separate 856 fields, each with their own $u
In master a link plus a youtube $u in a single 856 causes an error
In master as well, two regular links in a single 856 only seems to display the first link anyways. I think this is not a bug but intended functionality
I meant two separate 856$u fields. Will retest! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55091|0 |1 is obsolete| | --- Comment #9 from Claire Gravely <claire_gravely@hotmail.com> --- Created attachment 55093 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55093&action=edit Bug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links To test: Enable HTML5MediaYouTube and ensure WWW::YouTube::Download is installed Add an 856$u to a record like: https://www.youtube.com/watch?v=tu0qtEwb9gE Verify you can see the embedded player Use the youtube shortened like (from the share button) like: https://youtu.be/tu0qtEwb9gE Verify you can see the embedded player Sorry - all working as expected!! Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 55093 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55093 Bug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links Review of attachment 55093: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17231&attachment=55093) ----------------------------------------------------------------- ::: C4/HTML5Media.pm @@ +98,4 @@
# src if ( $HTML5Media_field->subfield('u') ) { $HTML5Media{srcblock} = $HTML5Media_field->subfield('u'); + if (grep /youtube|youtu[.]be/, $HTML5Media_field->subfield('u') ) {
/youtu\.?be/ seems easier to read, don't you think? @@ +109,5 @@
+ 'fmt' => '43' #webm is the only format compatible to all modern browsers. maybe check for available qualities + } + ); + }; + if ($@) { warn $@; }
What kind of errors do you handle here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 55298 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55298&action=edit Bug 17231 - (QA Followup) Nicer grep -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #10)
Comment on attachment 55093 [details] [review] /youtu\.?be/ seems easier to read, don't you think?
Yeah, I can get behind that
@@ +109,5 @@
+ 'fmt' => '43' #webm is the only format compatible to all modern browsers. maybe check for available qualities + } + ); + }; + if ($@) { warn $@; }
What kind of errors do you handle here? if an URL contains 'youtube' but isn't a www.youtube.com link the downloader will fail - this way it fails nicer - something like this: http://www.nytimes.com/topic/company/youtube
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Nick Clemens from comment #12)
@@ +109,5 @@
+ 'fmt' => '43' #webm is the only format compatible to all modern browsers. maybe check for available qualities + } + ); + }; + if ($@) { warn $@; }
What kind of errors do you handle here? if an URL contains 'youtube' but isn't a www.youtube.com link the downloader will fail - this way it fails nicer - something like this: http://www.nytimes.com/topic/company/youtube
So should not we test the regex from ^? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #13)
So should not we test the regex from ^?
Hmm...we could, but I would still argue for the eval - the downloader doesn't like links like: https://www.youtube.com/watch?v=kE2Akm55miw Preferable to fail on creating the link than die altogether I think -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55093|0 |1 is obsolete| | Attachment #55298|0 |1 is obsolete| | --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 55324 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55324&action=edit Bug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links To test: Enable HTML5MediaYouTube and ensure WWW::YouTube::Download is installed Add an 856$u to a record like: https://www.youtube.com/watch?v=tu0qtEwb9gE Verify you can see the embedded player Use the youtube shortened like (from the share button) like: https://youtu.be/tu0qtEwb9gE Verify you can see the embedded player Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 55325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55325&action=edit Bug 17231 - (QA Followup) Nicer grep Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Nick! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17231 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Status|Pushed to Master |Pushed to Stable --- Comment #18 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.04. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org