Closed Bug 1154894 Opened 5 years ago Closed 5 years ago

Disable test_plugin_default_state.js so Thunderbird can ship with plugins disabled by default

Categories

(Core :: Plug-ins, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: rkent, Assigned: rkent)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
An alternative would be to change the test so it quits early instead of failing if plugins are disabled by default. Not sure whether that would be more or less likely to get review than a change to xpcshell.ini...
My guess is that a change to xpcshell.ini that simply disabled the test for Thunderbird would be the easiest sell.
Attached patch Disable tests that need plugins (obsolete) — Splinter Review
This will disable the tests that are failing when bug 1138154 lands. This is an m-c patch.
Assignee: nobody → rkent
Status: NEW → ASSIGNED
Submit it for review?
Comment on attachment 8595615 [details] [diff] [review]
Disable tests that need plugins

Review of attachment 8595615 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/test/unit/xpcshell.ini
@@ +25,2 @@
>  [test_plugin_default_state_xpi.js]
> +skip-if = appname == "thunderbird"

You probably don't need to disable this test unless you plan to change the pref("plugin.defaultXpi.state", 2); in the patch in bug 1138154 (that keeps plugins bundled in addons enabled).
Component: Security → Plug-ins
Product: MailNews Core → Core
Version: 38 → Trunk
Attachment #8595615 - Attachment is obsolete: true
Attachment #8597056 - Flags: review?(john)
Comment on attachment 8597056 [details] [diff] [review]
Disable two tests

So the issue here is that the test is asserting that the default plugin state is not disabled - but the test is also testing other things, e.g. ensuring that the pref works. I'm not sure I see the value in the | defaultState == DISABLED | assertion, that could probably be removed and let the test continue to test the functionality of the pref on all apps.

I have little history here though, I think :bsmedberg would be the better person to ask on what our policy is WRT thunderbird and plugin tests
Attachment #8597056 - Flags: review?(john) → review?(benjamin)
Blocks: 1138154
Comment on attachment 8597056 [details] [diff] [review]
Disable two tests

I'm ok with these skip-ifs. I'd prefer to keep the default-state check for Firefox since we don't have any other check for this fairly important setting.
Attachment #8597056 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/21701097dafb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Blocks: 1165152
You need to log in before you can comment on or make changes to this bug.