[webkit-reviews] review denied: [Bug 103413] TextTrackCue's .endTime property should fire a TypeError when NaN is assigned : [Attachment 177486] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Antoine Quint
<graouts at apple.com>'s request for review:
Bug 103413: TextTrackCue's .endTime property should fire a TypeError when NaN
is assigned
https://bugs.webkit.org/show_bug.cgi?id=103413

Attachment 177486: Patch
https://bugs.webkit.org/attachment.cgi?id=177486&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=177486&action=review


> Source/WebCore/html/track/TextTrackCue.cpp:273
> +    if (WTF::double_conversion::Double(value).IsSpecial()) {

I don't see Double() being used to test this anywhere else in WebCore, and
while I don't know enough about it to know that it is a problem I would rather
we stick with the more commonly used isinf() and isnan(). Note that you don't
need to test for negative vs. positive infinity as inf() returns true for
either (eg. see MediaTime::createWithDouble).

> Source/WebCore/html/track/TextTrackCue.cpp:290
> +    if (WTF::double_conversion::Double(value).IsSpecial()) {

Ditto.

> LayoutTests/media/track/opera/interfaces/TextTrackCue/endTime-expected.txt:3
> +PASS TextTrackCue.endTime, script-created cue 
> +TIMEOUT TextTrackCue.endTime, parsed cue Test timed out

TIMEOUT? ChangeLog should note this if it is intentional.

>
LayoutTests/media/track/opera/interfaces/TextTrackCue/startTime-expected.txt:3
> +PASS TextTrackCue.startTime, script-created cue 
> +TIMEOUT TextTrackCue.startTime, parsed cue Test timed out

Ditto.