Summary: | TextTrackCue's .endTime property should fire a TypeError when NaN is assigned | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, eric.carlson, feature-media-reviews, gyuyoung.kim, ojan, rakuco, webkit-bug-importer, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | http://w3c-test.org/html/tests/submission/Opera/media/interfaces/TextTrackCue/endTime.html | ||||||||
Attachments: |
|
Description
Antoine Quint
2012-11-27 08:14:53 PST
We run into the same issue for .startTime as shown in http://w3c-test.org/html/tests/submission/Opera/media/interfaces/TextTrackCue/startTime.html. Created attachment 177486 [details]
Patch
Comment on attachment 177486 [details] Patch 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. (In reply to comment #4) > (From update of attachment 177486 [details]) > 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). Will adopt the isnan() and is infinite() methods from wtf/MathExtras.h in an updated patch. > > 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. It is intentional and the LayoutTests/ChangeLog mentioned the blocking bug (https://bugs.webkit.org/show_bug.cgi?id=103258). Created attachment 177737 [details]
Patch
Comment on attachment 177737 [details] Patch Clearing flags on attachment: 177737 Committed r136684: <http://trac.webkit.org/changeset/136684> All reviewed patches have been landed. Closing bug. |