fix: player.duration() should return NaN if duration is not known

[alex-barstow]

Description

player.duration() currently returns 0 if the duration is not known, when it should return NaN. The problem is that if NaN is passed to player.duration() as an argument, we set the duration to 0. If player.cache_.duration was set to NaN by other means, player.duration() would still return 0. If approved, this will require a sister PR in the 5.x branch as well.

Specific Changes proposed

Modify the player duration() method so that it will 1.) set the cached duration to NaN if it is passed in as an argument, and 2.) return the proper value when called without an argument.

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Change has been verified in an actual browser (Chome, Firefox, IE)
  • Unit Tests updated or fixed
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[gkatsev]

I think in here we still want to do || 0 because if what is passed in, isn't a number we should just assume 0.

[alex-barstow]

I think the problem with leaving the || 0 is that if player.techGet_('duration') equals NaN when handleTechDurationChange_() is first called, then the initial value of player.cache_.duration is 0 rather than NaN. At least that was the case with the page I was testing with.

[gkatsev]

Ah, I see. 🤔
I guess maybe this is fine since parseFloat() will return NaN for bad values. So, we'd just want to make sure the rest of this method handles NaN correctly, if it doesn't already.

[gkatsev]

Yeah, looks like this is fine but I think the duration display's updateDuration method needs to be updated to account for NaN values. Specifically, what happens if we start with unknown duration, transitioning from unknown duration to a known duration, and transitioning from a known duration to an unknown duration. That last one in particular I think might be problematic, though, not sure how common it is.

[alex-barstow]

This conditional will be false if player.duration() = NaN, so off the top of my head I can't think of a problem going from a unknown duration to a known duration. If we are transitioning from a known duration to an unknown duration, I believe the duration display (as currently written) would keep its previous 'known' state until the new duration becomes known and the display updates. Assuming I'm right about that, would this be acceptable behavior?

[gkatsev]

Yeah, that's probably fine. Just wanted someone to think about it in some more depth.

[gkatsev]

I realized one thing that this is missing: tests :)

[gkatsev]

@alex-barstow I pulled it in as is, but would be great to get some tests later, maybe when you get the 5.x PR ready.