The default bug view has changed. See this FAQ.

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\content-policy\test-plugins-policy.js test-plugin-blocked.js test-plugin-crashing.js test-plugin-outdated.js

RESOLVED FIXED in Thunderbird 52.0

Status

Thunderbird
General
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: Jorg K (GMT+1), Assigned: Jorg K (GMT+1))

Tracking

Trunk
Thunderbird 52.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 months ago
First seen Oct 4, 2016 (in fact on Daily of previous push) 

https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=9233be0faf407d84fa47c14db8e70d8e1002913b

https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=9068b163202c8a54819ddcca48ff934da6992172

Various plugin related Mozmill tests fail, quoting log:
https://archive.mozilla.org/pub/thunderbird/nightly/2016/10/2016-10-04-03-02-07-comm-central/comm-central_xp_ix_test-mozmill-bm112-tests1-windows-build16.txt.gz

EXCEPTION: plugin is null
at: test-plugin-blocked.js line 60
at: test-plugin-crashing.js line 78
at: test-plugin-outdated.js line 44

All these lines are preceded by:
let plugin = get_test_plugin();

So obviously this returns null now.
(Assignee)

Comment 1

6 months ago
Most likely caused by
https://hg.mozilla.org/mozilla-central/rev/fea530023849

Bug 1269807 - Remove support for all NPAPI plugins except for Flash, behind a pref. Tests that use the testplugin for now set the pref to keep it working. This will be disabled for ESR 52, but enabled for release 52. In the next cycle, the pref will be removed and this will be hardcoded. r=jimm

function allow_all_plugins() {
  Services.prefs.setBoolPref("plugin.load_flash_only", false);
}
(Assignee)

Comment 2

6 months ago
Created attachment 8797683 [details] [diff] [review]
1307516.patch

Let's see whether this works:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a71174930a94b9de5de33ae9d386ebb4524d095f
Assignee: nobody → jorgk
Attachment #8797683 - Flags: review?(acelists)

Comment 3

6 months ago
Note that there is a follow-up on this, bug 1307501 (meaning, the check for Flash plugin doesn't work either, thus right now *no* plugin is allowed and the tests mail still fail even if plugin.load_flash_only is set to false).
(Assignee)

Comment 4

6 months ago
That's not how I read it. With the preference set to true, apparently the Flash plugin does not run when it should; with the preference set to false, all plugins should run.

Comment 5

6 months ago
Oh, ok - you are right. The pref should bypass that check entirely.
(Assignee)

Comment 6

6 months ago
Comment on attachment 8797683 [details] [diff] [review]
1307516.patch

That didn't work at all :-((
Attachment #8797683 - Flags: review?(acelists)

Comment 7

6 months ago
Comment on attachment 8797683 [details] [diff] [review]
1307516.patch

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

Can we just remove all plugin support and remove all the tests? :)

Anyway, the fix seems to NOT work.

::: mail/test/mozmill/content-policy/test-plugins-policy.js
@@ +46,1 @@
>    composeHelper = collector.getModule('compose-helpers');

Please change the var setupmodule to 'function setupModule(module)'.

@@ +46,2 @@
>    composeHelper = collector.getModule('compose-helpers');
>    composeHelper.installInto(module);

This does not need to be a global variable.
(Assignee)

Comment 8

6 months ago
(In reply to :aceman from comment #7)
> Can we just remove all plugin support and remove all the tests? :)
So plugin is not equal add-on? I'd hate to remove add-on support ;-(
 
> Anyway, the fix seems to NOT work.
That's what I said in comment #6.
(Assignee)

Comment 9

6 months ago
(In reply to Jorg K (GMT+2) from comment #8)
> So plugin is not equal add-on? I'd hate to remove add-on support ;-(

Figured it out ;-)
https://developer.mozilla.org/en-US/Add-ons/Plugins
So PDF, Flash and Realplayer. Well, only Flash these days.

Comment 10

6 months ago
It is possible that the pref is checked much sooner in the app startup. It may be too late once mozmill passes control to out test code. The plugins are probably enumerated somewhere soon in the m-c startup code.

It seems to me the toolkit tests are all xpcshell.
(Assignee)

Comment 11

6 months ago
Yes, it occurred to me that setting the pref comes too late. I have to refresh locally to look into it further. I guess we'd have to set the pref somewhere in the Mozmill framework early on.
(Assignee)

Comment 12

6 months ago
Created attachment 8797952 [details] [diff] [review]
1307516-take2.patch

This will make
content-tabs\test-plugin-blocked.js
content-tabs\test-plugin-crashing.js
content-tabs\test-plugin-outdated.js
work.

Still to do:
content-policy\test-plugins-policy.js

content-policy doesn't have a prefs.js, so I need to check how that can be integrated.
Attachment #8797683 - Attachment is obsolete: true
(Assignee)

Comment 13

6 months ago
Created attachment 8797955 [details] [diff] [review]
1307516-take2.patch (v2).

Done. I ran it locally and it passed the test.
Attachment #8797952 - Attachment is obsolete: true
Attachment #8797955 - Flags: review?(acelists)

Comment 14

6 months ago
Comment on attachment 8797955 [details] [diff] [review]
1307516-take2.patch (v2).

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

Nice, works for me.
Attachment #8797955 - Flags: review?(acelists) → review+
(Assignee)

Comment 15

6 months ago
https://hg.mozilla.org/comm-central/rev/b2a89654fd4bef893adeee46b18c2c2bf6abb53c
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
(Assignee)

Updated

6 months ago
Duplicate of this bug: 1308722
You need to log in before you can comment on or make changes to this bug.