57728 2011-04-03 13:12:35 -0700 Improper use of &quot in video-can-play-type.html layout test 2011-04-06 11:52:41 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 nancy.piedra nancy.piedra abarth ap commit-queue eric.carlson eric webkit.review.bot oldest_to_newest 378575 0 nancy.piedra 2011-04-03 13:12:35 -0700 The layout test media/video-can-play-type.html uses the &quot entity reference improperly. See below: testExpected("video.canPlayType('video/mpeg; Codecs=&quot;avc1.4D400C&quot;')", "probably"); The &quot entity reference does not get decoded so the ContentType class cannot parse the codecs parameter. This only seems to cause an issue with the Qt MediaPlayer implementation as I noticed that no other implementation seems to look at the codecs parameter (but if they did use this parameter, they would see the same problem). An attempt to parse the &quot in the ContentType class was attempted here: https://bugs.webkit.org/show_bug.cgi?id=53275 But I think the entity reference should not have been used in this case and ContentType should not be required to decode it. To reproduce the issue, run the video-can-play-type.html test and check the value of contentType in the ContentType class constructor. 378582 1 88019 nancy.piedra 2011-04-03 13:47:05 -0700 Created attachment 88019 Patch for video-can-play-type layout test Patch to change "&quote;" to '\"' 378651 2 eric 2011-04-03 22:19:02 -0700 Is this "improper" use of &quot a useful test? 378744 3 nancy.piedra 2011-04-04 05:42:03 -0700 Most of the MediaPlayer implementations that I've looked at so far (except for Qt) only check for the presence of a 'codecs' parameter so no one has noticed that this isn't parsed properly. I think this misuse of &quot could be a good negative test. I'm going to be modifying the Qt implementation to properly handle the codecs parameter and at that point I'll add more tests for parsing the codecs parameter and can include a test for this misuse of &quot. I'll probably do that under one of these two bugs: https://bugs.webkit.org/show_bug.cgi?id=42094 https://bugs.webkit.org/show_bug.cgi?id=53275 In the meantime, this test should probably be fixed because it does not seem to be a negative test. 378879 4 88019 ap 2011-04-04 10:49:50 -0700 Comment on attachment 88019 Patch for video-can-play-type layout test I think that this is fine to land. Eric Carlson (the author of the test) is CC'ed on this bug, and can educate us if &quot; was used intentionally. 379362 5 88019 commit-queue 2011-04-05 00:31:06 -0700 Comment on attachment 88019 Patch for video-can-play-type layout test Clearing flags on attachment: 88019 Committed r82909: <http://trac.webkit.org/changeset/82909> 379363 6 commit-queue 2011-04-05 00:31:10 -0700 All reviewed patches have been landed. Closing bug. 379372 7 webkit.review.bot 2011-04-05 01:34:17 -0700 http://trac.webkit.org/changeset/82909 might have broken GTK Linux 32-bit Release 379418 8 nancy.piedra 2011-04-05 03:25:42 -0700 I checked the Linux Gtk 32-bit build and on the buildbot and it does not seem to have any failures right now. 380688 9 eric.carlson 2011-04-06 11:52:41 -0700 (In reply to comment #4) > (From update of attachment 88019 [details]) > I think that this is fine to land. Eric Carlson (the author of the test) is CC'ed on this bug, and can educate us if &quot; was used intentionally. It is not useful, and was not added intentionally. 88019 2011-04-03 13:47:05 -0700 2011-04-05 00:31:06 -0700 Patch for video-can-play-type layout test videocanplay.diff text/plain 1616 nancy.piedra SW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9D aGFuZ2VMb2cJKHJldmlzaW9uIDgyNzc4KQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTEtMDQtMDIgIE5hbmN5IFBpZWRyYSAgPG5h bmN5LnBpZWRyYUBub2tpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgSW1wcm9wZXIgdXNlIG9mICZxdW90IGluIHZpZGVvLWNhbi1wbGF5LXR5 cGUgbGF5b3V0IHRlc3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTU3NzI4CisKKyAgICAgICAgKiBtZWRpYS92aWRlby1jYW4tcGxheS10eXBlLmh0bWw6 CisKIDIwMTEtMDQtMDIgIERhbiBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KIAogICAgICAg ICBSZXZlcnRlZCByODI3NzUgZHVlIHRvIGNoYW5nZXMgaW4gPGRldGFpbHM+IHRlc3QgcmVzdWx0 cywgd2hpY2ggYXJlCkluZGV4OiBMYXlvdXRUZXN0cy9tZWRpYS92aWRlby1jYW4tcGxheS10eXBl Lmh0bWwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvbWVkaWEvdmlkZW8tY2FuLXBsYXktdHlw ZS5odG1sCShyZXZpc2lvbiA4Mjc3MykKKysrIExheW91dFRlc3RzL21lZGlhL3ZpZGVvLWNhbi1w bGF5LXR5cGUuaHRtbAkod29ya2luZyBjb3B5KQpAQCAtMTgsOCArMTgsOCBAQAogICAgICAgICAg ICAgdGVzdEV4cGVjdGVkKCJ2aWRlby5jYW5QbGF5VHlwZSgpIiwgIiIpOwogICAgICAgICAgICAg dGVzdEV4cGVjdGVkKCJ2aWRlby5jYW5QbGF5VHlwZSgndmlkZW8vJykiLCAiIik7CiAgICAgICAg ICAgICB0ZXN0RXhwZWN0ZWQoInZpZGVvLmNhblBsYXlUeXBlKCd2aWRlby9ibGFoYmxhaCcpIiwg IiIpOwotICAgICAgICAgICAgdGVzdEV4cGVjdGVkKCJ2aWRlby5jYW5QbGF5VHlwZSgndmlkZW8v bXBlZzsgQ29kZWNzPSZxdW90O2F2YzEuNEQ0MDBDJnF1b3Q7JykiLCAicHJvYmFibHkiKTsKLSAg ICAgICAgICAgIHRlc3RFeHBlY3RlZCgidmlkZW8uY2FuUGxheVR5cGUoJyAgICAgICAgVmlkZW8v TVA0IDsgQ09ERUNTPSZxdW90O21wNHYuMjAuOCwgbXA0YS40MC4yJnF1b3Q7JykiLCAicHJvYmFi bHkiKTsKKyAgICAgICAgICAgIHRlc3RFeHBlY3RlZCgidmlkZW8uY2FuUGxheVR5cGUoJ3ZpZGVv L21wZWc7IENvZGVjcz1cImF2YzEuNEQ0MDBDXCInKSIsICJwcm9iYWJseSIpOworICAgICAgICAg ICAgdGVzdEV4cGVjdGVkKCJ2aWRlby5jYW5QbGF5VHlwZSgnICAgICAgICBWaWRlby9NUDQgOyBD T0RFQ1M9XCJtcDR2LjIwLjgsIG1wNGEuNDAuMlwiJykiLCAicHJvYmFibHkiKTsKICAgICAgICAg ICAgIHRlc3RFeHBlY3RlZCgidmlkZW8uY2FuUGxheVR5cGUoJ2F1ZGlvL21wZWcnKSIsICJtYXli ZSIpOwogICAgICAgICAgICAgdGVzdEV4cGVjdGVkKCJ2aWRlby5jYW5QbGF5VHlwZSgnYXVkaW8v V2F2JykiLCAibWF5YmUiKTsKIAo=