| Summary: | Improper use of " in video-can-play-type.html layout test | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nancy Piedra <nancy.piedra> | ||||
| Component: | Tools / Tests | Assignee: | 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: |
|
||||||
Created attachment 88019 [details]
Patch for video-can-play-type layout test
Patch to change ""e;" to '\"'
Is this "improper" use of " a useful test? 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 " 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 ". 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. 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 " was used intentionally.
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> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/82909 might have broken GTK Linux 32-bit Release I checked the Linux Gtk 32-bit build and on the buildbot and it does not seem to have any failures right now. (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 " was used intentionally. It is not useful, and was not added intentionally. |
The layout test media/video-can-play-type.html uses the " entity reference improperly. See below: testExpected("video.canPlayType('video/mpeg; Codecs="avc1.4D400C"')", "probably"); The " 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 " 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.