RESOLVED WONTFIX Bug 96134
Mac: <audio> and <video> should send Do Not Track when appropriate 
https://bugs.webkit.org/show_bug.cgi?id=96134
Summary Mac: <audio> and <video> should send Do Not Track when appropriate
Jer Noble
Reported 2012-09-07 11:17:14 PDT
<audio> and <video> should send Do Not Track when appropriate
Attachments
Patch (34.59 KB, patch)
2012-09-07 11:40 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch (34.15 KB, patch)
2012-09-07 13:18 PDT, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch (34.69 KB, patch)
2012-09-07 15:49 PDT, Jer Noble 		eric.carlson: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jer Noble
Comment 1 2012-09-07 11:40:26 PDT
Created attachment 162834 [details] Patch
Eric Carlson
Comment 2 2012-09-07 12:17:45 PDT
Comment on attachment 162834 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162834&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:304 > if ([headerFields.get() count]) > [options.get() setObject:headerFields.get() forKey:@"AVURLAssetHTTPHeaderFieldsKey"]; > + > + if (player()->mediaPlayerClient()->mediaPlayerShouldSendDoNotTrackHTTPHeader()) > + [headerFields.get() setObject:@"1" forKey:@"DNT"]; You should set the DNT header field first or AVURLAssetHTTPHeaderFieldsKey won't be set in the dictionary if there isn't a referrer or a UA header. > LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:8 > + > + Nit: two blank lines? > LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:17 > + > + Ditto. > LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:18 > + error_log(var_export(headers_list(), true)); What does this do? > LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:20 > + > + Nit: two blank lines? > LayoutTests/http/tests/media/video-donottrack.html:11 > +<script src=../../media-resources/video-test.js></script> > +<script src=../../media-resources/media-file.js></script> > +<script> > + function runTest () { Nit: is there any reason this isn't in the <head>?
Jer Noble
Comment 3 2012-09-07 12:55:14 PDT
Comment on attachment 162834 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162834&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:304 >> + [headerFields.get() setObject:@"1" forKey:@"DNT"]; > > You should set the DNT header field first or AVURLAssetHTTPHeaderFieldsKey won't be set in the dictionary if there isn't a referrer or a UA header. Sure thing. >> LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:8 >> + > > Nit: two blank lines? I'll condense these. >> LayoutTests/http/tests/media/resources/video-donottrack-check-donottrack.php:18 >> + error_log(var_export(headers_list(), true)); > > What does this do? This was just for debugging purposes, so that run-webkit-httpd would spit out the entire outgoing header array before returning. I'll remove it. Thanks!
Jer Noble
Comment 4 2012-09-07 13:18:35 PDT
Created attachment 162852 [details] Patch Patch for the EWS bots.
Early Warning System Bot
Comment 5 2012-09-07 14:51:51 PDT
Comment on attachment 162852 [details] Patch Attachment 162852 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13772887
Jer Noble
Comment 6 2012-09-07 15:49:15 PDT
Created attachment 162889 [details] Patch Fix the Qt build.
Eric Carlson
Comment 7 2012-09-07 17:18:56 PDT
Comment on attachment 162889 [details] Patch This looks fine to me, but don't you have to skip the new test on some platforms?
WebKit Review Bot
Comment 8 2012-09-08 05:34:20 PDT
Comment on attachment 162889 [details] Patch Attachment 162889 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13794258 New failing tests: http/tests/media/video-donottrack.html
Jer Noble
Comment 9 2012-09-10 11:10:39 PDT
Committed r128081: <http://trac.webkit.org/changeset/128081>
Jer Noble
Comment 11 2012-09-10 16:39:59 PDT
Rolled out in <http://trac.webkit.org/changeset/128129>
Note You need to log in before you can comment on or make changes to this bug.