Closed Bug 881270 Opened 11 years ago Closed 11 years ago

Either the test or the code for channel type overrides in test_bug391728.html are incorrect

Categories

(Core Graveyard :: Plug-ins, defect, P2)

x86_64
Windows 7
defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla24

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(1 file)

test_bug391728.html has a bunch of tests for correct plugin behavior, but there are two tests which are not testing what they think they are testing: http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/content/base/test/chrome/file_bug391728_2.html#l40 These tests have an embed/object with a specified type, but the channel is another (unknown) type. According to the comment and the test code, the channel type is always supposed to be used in preference to the specified type. But according to johns on IRC: Jun 06 14:16:25 <johns> bsmedberg: "The mimetype of the actual data is supposed to be used in preference" only applies to object tags Jun 06 14:16:37 * bholley has quit (Quit: bholley) Jun 06 14:16:59 <johns> bsmedberg: http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsObjectLoadingContent.cpp#l1386 Jun 06 14:17:19 <johns> "eAllowPluginSkipChannel" applies to applet and embed, but applet has no URI attribute from which to derive a channel Jun 06 14:18:07 <johns> bsmedberg: Err, that link should be http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsObjectLoadingContent.cpp#l1455 Jun 06 14:18:49 <johns> bsmedberg: I'm not sure what the spec says about this, but that's how we've behaved for a long time Jun 06 14:20:07 <bsmedberg> johns: it doesn't work, if the test plugin is enabled we use that This test happens to pass currently because the test plugin is blocklisted at the time we run the test: when we run test_unknown, we haven't reset the .blocklisted flag from the prior test_blocked test. http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/content/base/test/chrome/test_bug391728.html#l157 This means that the codepath which looks for types ignores the specified type and only looks at the channel type. If you run this test with the test plugin not being blocklisted, it fails. These two plugin tests (plugin5 and plugin6) fail once bug 875454 is fixed because we no longer ignore blocklisted plugins when finding content types; we wait until ShouldPlay to determine the blocklist state. Since that patch only changes the behavior when the specified mime type is disabled or blocked, which seems like an edge case, I'd like to disable/TODO those two tests for now and deal with them here.
Attachment #760425 - Flags: review?(jschoenick)
Comment on attachment 760425 [details] [diff] [review] Disable the tests for channel type override since they aren't testing what they ought to be testing, and what they ought to be testing is probably broken (at least in the <object> case) r?johns Talked with bsmedberg on IRC about this, we should just delete these tests. Our explicit logic for choosing a type when we have a channel is described here: http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsObjectLoadingContent.cpp#l1455 We also now have a more exhaustive test on picking correct types: http://dxr.mozilla.org/mozilla-central/source/content/base/test/test_object.html As for the spec, it allows either interpretation for the object tag: > "If the user agent is configured to strictly obey Content-Type headers for this resource [use the header type] [...] If there is a type attribute present on the object element, and that attribute's value is not a type that the user agent supports, but it is a type that a plugin supports, then let the resource type be the type specified in that type attribute" While requiring we use the type attribute for embed: > If the element has a type attribute, and that attribute's value is a type that a plugin supports, then the value of the type attribute is the content's type. So presumptive r+ from me on just removing these.
Attachment #760425 - Flags: review?(jschoenick)
Assignee: nobody → benjamin
Priority: -- → P2
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: