Closed Bug 1307516 Opened 5 years ago Closed 5 years ago

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

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 52.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
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);
}
Attached patch 1307516.patch (obsolete) — Splinter Review
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)
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).
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.
Oh, ok - you are right. The pref should bypass that check entirely.
Comment on attachment 8797683 [details] [diff] [review]
1307516.patch

That didn't work at all :-((
Attachment #8797683 - Flags: review?(acelists)
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.
(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.
(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.
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.
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.
Attached patch 1307516-take2.patch (obsolete) — Splinter Review
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
Done. I ran it locally and it passed the test.
Attachment #8797952 - Attachment is obsolete: true
Attachment #8797955 - Flags: review?(acelists)
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+
https://hg.mozilla.org/comm-central/rev/b2a89654fd4bef893adeee46b18c2c2bf6abb53c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
Duplicate of this bug: 1308722
You need to log in before you can comment on or make changes to this bug.