Cross-Browser HTML5 Audio

Hi Rbenhase,

I did a manual review of your code and found a some of issues.

Remove LICENSE.txt as it will be added automatically.

Try not to exceed more than 80 characters per line.

Try not to use HTML Tags within t() function (html5_audio.module on line 31), check this .

I will update you about the automated review later as the site is down.

Regards
M

Hey rbenhase,

I can confirm that you have corrected the usage of t() function.

I could make an automated review of your project, here is the link - http://ventral.org/pareview/httpgitdrupalorgsandboxrbenhase1674986git

Here are some tips :
- As i said before try to keep less than 80 characters on a line in any file.
- Never use ?> in any file of drupal module.
- Remove $Id from .info file.
- Check for indentations and whitespaces. There must be no whitespaces at the end of the line. Don't use tabs, use space.
- End the files with new line character (If using Linux, you can use "dos2unix" command to convert the file to linux format).
- Never work on master branch, create a branch 7.x-1.x and place all the code in that. The master branch must contain only
a README.txt that tells the developers about the correct branch to look for the code. The content can be found here.

After committing the changes you can always do a repeat review on ventral.org to check for errors.

Regards,
M

I'm sorry for the delay! There are about 100 other applications waiting for review and only a hand full of active reviewers.
We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

Hi !

I a did a manual review of your module. I am not an expert, below is what i found:

In your implementation of hook_help():
- The help path case 'admin/modules#description': seems incorrect, shoud be case 'admin/help#html5_audio':
- You should use return instead of $output. See http://api.drupal.org/api/drupal/modules!help!help.api.php/function/hook...
- In order to make your content translatable, you should use the t() function (without the html tag)
- Avoid css tag if possible

Very little suggestion: since the aim of the plugin used in this module is designed to make the audio player work in most broswers, i think you should precise in your readme file and in your project page which browser is supported.

I tested the module, it didn't work the first time.

I looked into your html5_audio_init.js, and i use Drupal Behavior (http://drupal.org/node/756722), then it worked:

(function ($) {

  Drupal.behaviors.html5_audio = {

    attach: function(context) {

    audiojs.events.ready(function() {
    var as = audiojs.createAll();
  });

   }

  };

  })(jQuery);

Hope that helps

Very cool module

Edit: one last thing, you could also check if the js plugin is present with hook_requirements() in your .install file

Closing due to lack of activity. Feel free to reopen if you are still working on this application.