Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Libraries
    • Labels:
    • Testing Instructions:
      Hide

      1/ get some mp3s and flash videos
      2/ create some activities that use these media files
      3/ enable media embedding everywhere
      4/ test using
      4a/ browser with flash
      4b/ browser without flash
      5c/ mobile browser

      Make sure the video/audio embedding works as before.

      Show
      1/ get some mp3s and flash videos 2/ create some activities that use these media files 3/ enable media embedding everywhere 4/ test using 4a/ browser with flash 4b/ browser without flash 5c/ mobile browser Make sure the video/audio embedding works as before.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w36_MDL-35185_m24_flowplayer
    • Rank:
      43822

      Activity

      Hide
      Eloy Lafuente (stronk7) added a comment - - edited

      wow, what a mess of versions, each component has its own (controls, audio, js, flash...)

      Just for confirmation:

      1) are the 2 hacks (do not load if no flash and no splashscreen) applied to the complete and min versions? I've been able to see the splash one, but not the conditional load of flash. Or perhaps that's not a flowplayer hack after all?

      2) What's the reason to change "M.cfg.jsrev == -10" to "-1" ? To serve the file directly and annihilate caches? Was that "-10" used anywhere and it has become "-1" there recently ? Also, why are you changing from "min" to "complete" there? After all, the rest of code uses always the "min" one, isn't it?

      Ciao

      Show
      Eloy Lafuente (stronk7) added a comment - - edited wow, what a mess of versions, each component has its own (controls, audio, js, flash...) Just for confirmation: 1) are the 2 hacks (do not load if no flash and no splashscreen) applied to the complete and min versions? I've been able to see the splash one, but not the conditional load of flash. Or perhaps that's not a flowplayer hack after all? 2) What's the reason to change "M.cfg.jsrev == -10" to "-1" ? To serve the file directly and annihilate caches? Was that "-10" used anywhere and it has become "-1" there recently ? Also, why are you changing from "min" to "complete" there? After all, the rest of code uses always the "min" one, isn't it? Ciao
      Hide
      Petr Škoda added a comment - - edited

      1/ both hacks are there, the second one is not visible in the diff because I changed the whitespace in the first one only; I expanded the min version in PhpStorm to reapply the patches and then manually injected them to the minimised version - ugly ugly ugly

      2/ yes, "- 1" version means "do not cache", I do not know how the "- 10" got there, but it works fine with "- 1" now, I tested it while unsuccessfully trying to remove the 2 hacks, you can try it yourself by setting "$CFG->cachejs" = 1; or to 0; the "&rev=' + M.cfg.jsrev;" is not really necessary there, I just did not want to change everything...

      Show
      Petr Škoda added a comment - - edited 1/ both hacks are there, the second one is not visible in the diff because I changed the whitespace in the first one only; I expanded the min version in PhpStorm to reapply the patches and then manually injected them to the minimised version - ugly ugly ugly 2/ yes, "- 1" version means "do not cache", I do not know how the "- 10" got there, but it works fine with "- 1" now, I tested it while unsuccessfully trying to remove the 2 hacks, you can try it yourself by setting "$CFG->cachejs" = 1; or to 0; the "&rev=' + M.cfg.jsrev;" is not really necessary there, I just did not want to change everything...
      Hide
      Eloy Lafuente (stronk7) added a comment -

      Integrated (24 only), thanks!

      PS: And the change from min to complete when no caching is enabled... is to show devs the code in a readable way, correct?

      Show
      Eloy Lafuente (stronk7) added a comment - Integrated (24 only), thanks! PS: And the change from min to complete when no caching is enabled... is to show devs the code in a readable way, correct?
      Hide
      Petr Škoda added a comment -

      Yes, the expanded code is good for debugging.

      Show
      Petr Škoda added a comment - Yes, the expanded code is good for debugging.
      Hide
      Frédéric Massart added a comment -

      This test might not be failed, but I'd like to report what I have been through.

      With Flash plugin enabled, I could:

      • Always play the FLV video
      • Randomely play the MP3. Chrome, Firefox don't always play the MP3, although the inspector shows that the file is being downloaded (Ubuntu issue?), I did not have any issue on Opera. At first I thought that specials chars in the MP3 file name could lead to issues, but then sometimes I could play those files with spaces and &.

      Without Flash plugin enabled:

      • Tested on Safari iPad iOS 5. The MP3 could be played, where as the FLV was just a link.
      • Tested on Chrome and Firefox. The FLV displayed a link on both browsers. On Firefox the MP3 could be played, on Chrome it did not work. The player did not seem to react when I clicked 'Play'.

      I also noticed that some elements of TinyMCE popups and File Picker are displayed under the Flash object. (See screenshots)

      This issue might not be responsible for those, in such case we can pass it.

      Show
      Frédéric Massart added a comment - This test might not be failed, but I'd like to report what I have been through. With Flash plugin enabled, I could: Always play the FLV video Randomely play the MP3. Chrome, Firefox don't always play the MP3, although the inspector shows that the file is being downloaded (Ubuntu issue?), I did not have any issue on Opera. At first I thought that specials chars in the MP3 file name could lead to issues, but then sometimes I could play those files with spaces and &. Without Flash plugin enabled: Tested on Safari iPad iOS 5. The MP3 could be played, where as the FLV was just a link. Tested on Chrome and Firefox. The FLV displayed a link on both browsers. On Firefox the MP3 could be played, on Chrome it did not work. The player did not seem to react when I clicked 'Play'. I also noticed that some elements of TinyMCE popups and File Picker are displayed under the Flash object. (See screenshots) This issue might not be responsible for those, in such case we can pass it.
      Hide
      Petr Škoda added a comment -

      I do not understand, does it work the same as before? Did you try to switch to current dev master?

      Show
      Petr Škoda added a comment - I do not understand, does it work the same as before? Did you try to switch to current dev master?
      Hide
      Frédéric Massart added a comment -

      Good point, I should have tried on master. The behaviours are similar, so I guess this issue can be passed.

      Show
      Frédéric Massart added a comment - Good point, I should have tried on master. The behaviours are similar, so I guess this issue can be passed.
      Hide
      Petr Škoda added a comment -

      Restarted, thanks.

      Show
      Petr Škoda added a comment - Restarted, thanks.
      Hide
      Eloy Lafuente (stronk7) added a comment -

      Many thanks for the hard work.

      These changes have been spread upstream and are already available in the git and cvs repositories.

      Ciao

      Show
      Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao
      Hide
      Michael de Raadt added a comment -

      It looks like the previous Flowplayer version is buggy with the new version of Flash. We might consider backporting this new player.

      Show
      Michael de Raadt added a comment - It looks like the previous Flowplayer version is buggy with the new version of Flash. We might consider backporting this new player.

        People

        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved: