WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
103413
TextTrackCue's .endTime property should fire a TypeError when NaN is assigned
https://bugs.webkit.org/show_bug.cgi?id=103413
Summary
TextTrackCue's .endTime property should fire a TypeError when NaN is assigned
Antoine Quint
Reported
2012-11-27 08:14:53 PST
We're failing the Opera-submitted test at
http://w3c-test.org/html/tests/submission/Opera/media/interfaces/TextTrackCue/endTime.html
due to not triggering a TypeError when trying to set .endTime to NaN, which is not an appropriate value.
Attachments
Patch
(16.15 KB, patch)
2012-12-04 08:16 PST
,
Antoine Quint
no flags
Details
Formatted Diff
Diff
Patch
(16.02 KB, patch)
2012-12-05 06:20 PST
,
Antoine Quint
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2012-11-27 08:15:07 PST
<
rdar://problem/12758008
>
Antoine Quint
Comment 2
2012-11-27 08:29:42 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
.
Antoine Quint
Comment 3
2012-12-04 08:16:51 PST
Created
attachment 177486
[details]
Patch
Eric Carlson
Comment 4
2012-12-04 08:54:22 PST
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.
Antoine Quint
Comment 5
2012-12-05 06:11:42 PST
(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
).
Antoine Quint
Comment 6
2012-12-05 06:20:43 PST
Created
attachment 177737
[details]
Patch
WebKit Review Bot
Comment 7
2012-12-05 07:26:16 PST
Comment on
attachment 177737
[details]
Patch Clearing flags on attachment: 177737 Committed
r136684
: <
http://trac.webkit.org/changeset/136684
>
WebKit Review Bot
Comment 8
2012-12-05 07:26:21 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug