Bug 113615 - Update all float attributes in HTMLMediaElement to double
Summary: Update all float attributes in HTMLMediaElement to double
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-03-29 15:21 PDT by Aaron Colwell
Modified: 2013-04-10 09:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch (71.18 KB, patch)
2013-03-29 16:57 PDT, Aaron Colwell
no flags Details | Formatted Diff | Diff
Remove ChangeLog cruft and fix Mac build (69.54 KB, patch)
2013-03-29 17:57 PDT, Aaron Colwell
no flags Details | Formatted Diff | Diff
Another mac build fix. (69.58 KB, patch)
2013-03-29 21:08 PDT, Aaron Colwell
no flags Details | Formatted Diff | Diff
Fix unused parameter error (69.58 KB, patch)
2013-03-30 00:10 PDT, Aaron Colwell
buildbot: commit-queue-
Details | Formatted Diff | Diff
Proposed patch (66.82 KB, patch)
2013-04-10 08:16 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Colwell 2013-03-29 15:21:38 PDT
HTMLMediaElement has several attributes declared as float that are actually specified as double in the HTML5 spec. These include currentTime, startTime, duration, defaultPlaybackRate, playbackRate, and volume. These should be updated to match the HTML5 spec.
Comment 1 Aaron Colwell 2013-03-29 16:57:04 PDT
Created attachment 195821 [details]
Patch
Comment 2 Eric Carlson 2013-03-29 17:09:17 PDT
Comment on attachment 195821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195821&action=review

This looks fine, but please wait until the bots have had their way with it before landing. Also, please file bugs against all ports that need to be updated, and also file one to remove the old versions once everyone has updated.

> Source/WebCore/ChangeLog:41
> +        (HTMLMediaElement):

These bogus entries generated by prepare-ChangeLog aren't helpful.
Comment 3 Build Bot 2013-03-29 17:29:50 PDT
Comment on attachment 195821 [details]
Patch

Attachment 195821 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17257710
Comment 4 Build Bot 2013-03-29 17:38:41 PDT
Comment on attachment 195821 [details]
Patch

Attachment 195821 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17311762
Comment 5 Aaron Colwell 2013-03-29 17:57:48 PDT
Created attachment 195831 [details]
Remove ChangeLog cruft and fix Mac build
Comment 6 Build Bot 2013-03-29 18:04:32 PDT
Comment on attachment 195831 [details]
Remove ChangeLog cruft and fix Mac build

Attachment 195831 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17356171
Comment 7 Build Bot 2013-03-29 18:40:48 PDT
Comment on attachment 195831 [details]
Remove ChangeLog cruft and fix Mac build

Attachment 195831 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17255683
Comment 8 Aaron Colwell 2013-03-29 21:08:22 PDT
Created attachment 195838 [details]
Another mac build fix.
Comment 9 Build Bot 2013-03-29 21:51:12 PDT
Comment on attachment 195838 [details]
Another mac build fix.

Attachment 195838 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17377035
Comment 10 Build Bot 2013-03-29 21:58:07 PDT
Comment on attachment 195838 [details]
Another mac build fix.

Attachment 195838 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17345825
Comment 11 Aaron Colwell 2013-03-30 00:10:31 PDT
Created attachment 195845 [details]
Fix unused parameter error
Comment 12 Build Bot 2013-03-30 00:17:08 PDT
Comment on attachment 195845 [details]
Fix unused parameter error

Attachment 195845 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17317297
Comment 13 Build Bot 2013-03-30 01:01:18 PDT
Comment on attachment 195845 [details]
Fix unused parameter error

Attachment 195845 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17250676
Comment 14 Aaron Colwell 2013-03-30 16:44:27 PDT
Eric, could you help me with the Mac build errors. I'm getting these symbol export errors.

Undefined symbols for architecture x86_64:
  "__ZN7WebCore16HTMLMediaElement14setCurrentTimeEfRi", referenced from:
     -exported_symbol[s_list] command line option
  "__ZN7WebCore16HTMLMediaElement9setVolumeEfRi", referenced from:
     -exported_symbol[s_list] command line option
  "__ZN7WebCore16HTMLMediaElement6rewindEf", referenced from:
     -exported_symbol[s_list] command line option
ld: symbol(s) not found for architecture x86_64

I'm assuming I have to change Source/WebCore/WebCore.order and/or Source/WebCore/WebCore.exp.in, but I don't have a Mac build environment so I have no idea what the new mangled signatures should be.
Comment 15 Eric Carlson 2013-04-10 08:16:40 PDT
Created attachment 197271 [details]
Proposed patch
Comment 16 Geoffrey Garen 2013-04-10 08:31:04 PDT
Comment on attachment 197271 [details]
Proposed patch

r=me

It would be nice to write a test that confirmed the additional precision. For example, you could seek to ".0000000000000000000000000000000000000000000001" and then ask for .currentTime and confirm that it's not 0.
Comment 17 Eric Carlson 2013-04-10 09:02:28 PDT
(In reply to comment #16)
> (From update of attachment 197271 [details])
> r=me
> 
> It would be nice to write a test that confirmed the additional precision. For example, you could seek to ".0000000000000000000000000000000000000000000001" and then ask for .currentTime and confirm that it's not 0.

That won't work reliably because currentTime is the value returned by the media engine, and it will be clamped to whatever level of precision it supports.
Comment 18 Radar WebKit Bug Importer 2013-04-10 09:25:26 PDT
<rdar://problem/13620078>
Comment 19 Eric Carlson 2013-04-10 09:28:06 PDT
https://trac.webkit.org/r148099