i think the filenames need some tweaking otherwise this is looking strong :)

thx for the audio test changes :)

My pleasure! :)

I'll try tweaking the filenames up a little sooon.

I saw these the other day and was going to ask you to do a pull request, awesome work.

The test names should probably all be lowercase to be inline with the others plugins (we should document that in docs on what is preferred?).

I'm getting errors with the apng test not liking the following line because of the preceding cast to bool operator being outside the parentheses.

if !! (typeof canvas.getContext == 'undefined') {

Also since this is an async test I'd put the the Modernizr.addTest call inside the onload event. The same way the webp plugin does it.

@ryanseddon How does the updated PR look?

There are a few different ways the async test could be done, but I figured that wrapping the load/onerror event handler attachment inside the context test would avoid a little overhead. If you have a preferred alternative structure to it, let me know and I'll happily update further.

Cheers!

As requested by @paulirish, I've updated the PR to include the additional tests I added to the other repo today.

(thanks to @mathiasbynens and @timmywil for the head's up on naming)

Awesome thank you. :)

Looks good.

What I was saying about the async test was to do the addTest inside the onload as that covers both passing and failing. I'm not sure if the onerror will ever fire in any browser?

As @ryanseddon said:

The test names should probably all be lowercase to be inline with the others plugins (we should document that in docs on what is preferred?).

All tests on http://modernizr.github.com/Modernizr/test/ have lowercase names.

addTest toLowerCase()s feature names: addTest("featureName", bool) => Modernizr.featurename === bool

@davidmurdoch Exactly; it would be much less confusing to lowercase the test names manually for people reading the feature test source code.

Agreed.

aside: addTest should probably not enforce casing for feature names at all. The "official" Modernizr policy could be lowercase - but devs should be free to use whatever convention they like. Right? (Line 863 as well as addTest would need to be updated).

Sounds good. I'll update the PR today to fall in line with the letter casing in other tests :)

Sent from my iPad

On 2 Dec 2011, at 09:50, David Murdochreply@reply.github.com wrote:

Agreed.

aside: addTest should probably not enforce casing for feature names at all. The "official" Modernizr policy could be lowercase - but devs should be free to use whatever convention they like. Right? (Line 863 as well as addTest would need to be updated).

---

Reply to this email directly or view it on GitHub:
#419 (comment)

Cases updated <3

can someone review?

im gonna tweak these filenames a touch more and kill some superfluous !! casting since we're using in operator.
followups coming..

but THANK YOU Addy! great stuff. I think this kills a few outstanding things in the issue tracker too.

My pleasure @paulirish! Glad they could come in useful for something :)