Bug 96134

Summary:

Mac: <audio> and <video> should send Do Not Track when appropriate

Product:

WebKit

Reporter:

Jer Noble <jer.noble>

Component:

New Bugs

Assignee:

Jer Noble <jer.noble>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

abecsi, cmarcelo, dglazkov, eric.carlson, feature-media-reviews, japhet, menard, webkit.review.bot, zoltan

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	eric.carlson: review+, webkit.review.bot: commit-queue-

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	Details Formatted Diff Diff
Patch (34.15 KB, patch) 2012-09-07 13:18 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch (34.69 KB, patch) 2012-09-07 15:49 PDT, Jer Noble	eric.carlson: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
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 10 2012-09-10 11:15:25 PDT

Skipped the new test on & filed the following bugs for other platforms: Chromium: https://bugs.webkit.org/show_bug.cgi?id=96287 GTK: https://bugs.webkit.org/show_bug.cgi?id=96290 Qt: https://bugs.webkit.org/show_bug.cgi?id=96291 EFL: https://bugs.webkit.org/show_bug.cgi?id=96292 Win: https://bugs.webkit.org/show_bug.cgi?id=96293

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.