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)
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.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #760425 -
Flags: review?(jschoenick)
Comment 2•11 years ago
|
||
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 | ||
Updated•11 years ago
|
Assignee: nobody → benjamin
Priority: -- → P2
Comment 3•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•