Bug 57728

Summary: Improper use of &quot in video-can-play-type.html layout test
Product: WebKit Reporter: Nancy Piedra <nancy.piedra>
Component: Tools / TestsAssignee: Nancy Piedra <nancy.piedra>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, commit-queue, eric.carlson, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch for video-can-play-type layout test none

Nancy Piedra
Reported 2011-04-03 13:12:35 PDT
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.
Attachments
Patch for video-can-play-type layout test (1.58 KB, patch)
2011-04-03 13:47 PDT, Nancy Piedra 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Nancy Piedra
Comment 1 2011-04-03 13:47:05 PDT
Created attachment 88019 [details] Patch for video-can-play-type layout test Patch to change "&quote;" to '\"'
Eric Seidel (no email)
Comment 2 2011-04-03 22:19:02 PDT
Is this "improper" use of &quot a useful test?
Nancy Piedra
Comment 3 2011-04-04 05:42:03 PDT
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.
Alexey Proskuryakov
Comment 4 2011-04-04 10:49:50 PDT
Comment on attachment 88019 [details] 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.
WebKit Commit Bot
Comment 5 2011-04-05 00:31:06 PDT
Comment on attachment 88019 [details] Patch for video-can-play-type layout test Clearing flags on attachment: 88019 Committed r82909: <http://trac.webkit.org/changeset/82909>
WebKit Commit Bot
Comment 6 2011-04-05 00:31:10 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 7 2011-04-05 01:34:17 PDT
http://trac.webkit.org/changeset/82909 might have broken GTK Linux 32-bit Release
Nancy Piedra
Comment 8 2011-04-05 03:25:42 PDT
I checked the Linux Gtk 32-bit build and on the buildbot and it does not seem to have any failures right now.
Eric Carlson
Comment 9 2011-04-06 11:52:41 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.