Complete loss of Mozmill testing - Exception: Sorry, cannot connect to jsbridge extension, port 24242 - 2018-03-27

RESOLVED FIXED in Thunderbird 61.0

Status

defect
--
blocker
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jorgk, Assigned: jorgk)

Tracking

({regression})

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 22 obsolete attachments)

1.05 KB, patch
jorgk
: review+
Details | Diff | Splinter Review
138.08 KB, patch
aceman
: review+
Details | Diff | Splinter Review
5.26 KB, patch
aceman
: review+
Details | Diff | Splinter Review
8.25 KB, patch
aceman
: review+
Details | Diff | Splinter Review
1.19 KB, patch
aceman
: review+
Details | Diff | Splinter Review
7.64 KB, patch
aceman
: review+
Details | Diff | Splinter Review
(Assignee)

Description

a year ago
Log
https://taskcluster-artifacts.net/DvMJhEBGTRiIcuhXZevqhA/0/public/logs/live_backing.log
says:

INFO -  1522146643444	addons.xpi-utils	WARN	addMetadata: Add-on jsbridge@mozilla.com is invalid: Error: Non-restartless extensions no longer supported (resource://gre/modules/addons/XPIInstall.jsm:531:15) JS Stack trace: loadManifestFromRDF@XPIInstall.jsm:531:15

INFO -  1522146643454	addons.xpi-utils	WARN	addMetadata: Add-on mozmill@mozilla.com is invalid: Error: Non-restartless extensions no longer supported (resource://gre/modules/addons/XPIInstall.jsm:531:15) JS Stack trace: loadManifestFromRDF@XPIInstall.jsm:531:15

INFO -  1522146643561	addons.xpi-utils	WARN	addMetadata: Add-on specialpowers@mozilla.org is invalid: Error: Invalid addon ID: expected addon ID specialpowers@mozilla.org, found special-powers@mozilla.org in manifest (resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1150:15) JS Stack trace: addMetadata@XPIProviderUtils.js:1150:15

INFO -  1522146643562	addons.xpi-utils	WARN	Could not uninstall invalid item from locked install location
03:30:43     INFO -  1522146643573	addons.xpi-utils	WARN	addMetadata: Add-on {a62ef8ec-5fdc-40c2-873c-223b8a6925cc} is invalid: Error: Non-restartless extensions no longer supported (resource://gre/modules/addons/XPIInstall.jsm:531:15) JS Stack trace: loadManifestFromRDF@XPIInstall.jsm:531:15

INFO -  1522146643573	addons.xpi-utils	WARN	Could not uninstall invalid item from locked install location
03:30:43     INFO -  1522146643575	addons.xpi-utils	WARN	addMetadata: Add-on {e2fda1a4-762b-4020-b5ad-a41df1933103} is invalid: Error: Non-restartless extensions no longer supported (resource://gre/modules/addons/XPIInstall.jsm:531:15) JS Stack trace: loadManifestFromRDF@XPIInstall.jsm:531:15

And more.

M-C last good: de32269720d056972b85f4eec5f0a8286d
M-C first bad: 97cdd8febc40ac6025bce5dec9f8dadb8e
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=de32269720d056972b85f4eec5f0a8286d&tochange=97cdd8febc40ac6025bce5dec9f8dadb8e

Bug 1447831 as expected. I'm surprised that Calendar still loads, but maybe that's packaged.

We can stop development now since we don't have test coverage :-(
(Assignee)

Comment 1

a year ago
Richard reports that manual installation of add-ons doesn't work any more, I guess that refers to non-restartless ones.
Blocks: 1447831
Keywords: regression
(Assignee)

Comment 2

a year ago
BTW, a62ef8ec-5fdc-40c2-873c-223b8a6925cc is Gdata and e2fda1a4-762b-4020-b5ad-a41df1933103 is Lightning. So even of we can get the Mozmill tests going again, we won't be able to run the ones for Lightning.

Curiously enough Lightning's Xpcshell tests haven't failed so far.
(In reply to Jorg K (GMT+1) from comment #2)
> Curiously enough Lightning's Xpcshell tests haven't failed so far.

That seems to suggest that this code still works: https://searchfox.org/comm-central/source/calendar/test/unit/head_consts.js#22
Or that Lightning isn't loaded. In my local build it's not shown in the add-on manager but it's still in the extensions directory.
(Assignee)

Comment 5

a year ago
It is loaded, otherwise all the Xpcshell tests would fail. It's not loaded in Mozmill runs, see comment #2.
(Assignee)

Comment 6

a year ago
Patrick, could you assist us in making Mozmill and JSBridge restart-less. There doesn't appear to be a whole lot of code involved. The overlays seem trivial as they just include overlay.js which we can include in the overlaid files.

I'd say that's more urgent than the generic overlay loader, since we really need Mozmill testing back asap.

The source is here:
https://dxr.mozilla.org/comm-central/source/mail/test/resources/jsbridge/jsbridge/extension
https://dxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/mozmill/extension

Or maybe we can share the work if there's a recipe.
Flags: needinfo?(patrick)
Yes, I agree - this is more urgent than anything else.

This is the general description of Bootstrapped add-ons:
https://developer.mozilla.org/en-US/docs/Archive/Add-ons/Bootstrapped_extensions

And the following article describes every step that needs to be done (your recipe):
https://developer.mozilla.org/en-US/docs/Archive/Add-ons/How_to_convert_an_overlay_extension_to_restartless

I can look into it this evening, but I don't have much time today and tomorrow. I'd be happy if we could share the work. I'll ping you on IRC in the late afternoon.
Flags: needinfo?(patrick)
(Assignee)

Comment 8

a year ago
Interesting reading ;-) Many of the steps won't be necessary for Mozmill/JSBridge, I wonder how many will be necessary.
(Assignee)

Updated

a year ago
Blocks: 1448221
Posted patch Patch v1, first quick hack (obsolete) β€” β€” Splinter Review
I hacked in a quick version that brings back mozimill to the Tools menu in the messenger window. This is more to demonstrate how it's done.

The Mozmill window appears again, but I don't know how to run Mozmill tests :-/

Can you work on that?
Flags: needinfo?(jorgk)
(Assignee)

Comment 10

a year ago
Thanks, I'll look into it tonight. Surely the overlays will have to disappear from the manifest
https://dxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/mozmill/extension/chrome.manifest
and JSBridge needs similar treatment.

I never knew that you can install the add-on in TB, we only ever use it when running tests. That's easy:

cd obj*
mozmake SOLO_TEST=composition/test-image-display.js mozmill-one
(that a quick test, we have ones that run for ages).
Flags: needinfo?(jorgk)
(Assignee)

Comment 11

a year ago
Posted patch 1449149-jsbridge.patch (obsolete) β€” β€” Splinter Review
This is a copy of Patrick's patch slightly tweaked for JS Bridge.

Running my favourite test
mozmake SOLO_TEST=composition/test-image-display.js mozmill-one
I see:
JavaScript error: chrome://jsbridge/content/overlay.js, line 38: NS_ERROR_NOT_AVAILABLE:
JavaScript error: chrome://mozmill/content/overlay.js, line 39: NS_ERROR_NOT_AVAILABLE:

which refer to these lines:
var mozmillInit = {}; ChromeUtils.import('resource://mozmill/modules/init.js', mozmillInit);
var jsbridgeInit = {}; ChromeUtils.import('resource://jsbridge/modules/init.js',jsbridgeInit);
Resource:// urls are no longer supported (as I reported a few days ago). You need to register the files elsewhere, for example in chrome or via a proxy.
(Assignee)

Comment 13

a year ago
Yes, I'm reading that post and I'm tying to make sense of it. You wrote:

I fixed the resource:// issue by moving everything to
chrome://.../content/... . Not nice, but working.

Maybe we can chat briefly.
(Assignee)

Comment 14

a year ago
Posted patch 1449149-jsbridge-move.patch (obsolete) β€” β€” Splinter Review
This moves stuff in JS Bridge around to solve the
JavaScript error: chrome://jsbridge/content/overlay.js, line 38: NS_ERROR_NOT_AVAILABLE:
error. I'll have to do the same for Mozmill, only that that has more resource:// references.

Patrick, does this look correct?
Attachment #8963311 - Flags: feedback?(patrick)
(Assignee)

Comment 15

a year ago
Posted patch 1449149-mozmill-move.patch (obsolete) β€” β€” Splinter Review
This moves stuff in Mozmill around. After that, the test starts to run and I see:
Warning: unrecognized command line flag -jsbridge
Warning: unrecognized command line flag -foreground

In the end I get:
Exception: Sorry, cannot connect to jsbridge extension, port 24242

So both add-ons are restartless now and seem to get loaded successfully but the test doesn't work.
(Assignee)

Comment 16

a year ago
Posted patch 1449149-mozmill.patch (obsolete) β€” β€” Splinter Review
Same as Patrick's only with commit message and consistent filename.
Attachment #8963225 - Attachment is obsolete: true
(Assignee)

Comment 17

a year ago
Posted patch 1449149-mozmill-move.patch (v2) (obsolete) β€” β€” Splinter Review
Oops, sed hit some jquery file. Reverting that change.
Attachment #8963334 - Attachment is obsolete: true
(Assignee)

Comment 18

a year ago
I've confirmed with some debug that the restartless boilerplate is working. Upon closer inspection, it's very similar to what I have in my only restartless add-on "Dictionary for Recipient".

I guess we will have to add
  chrome://messenger/content/messengercompose/messengercompose.xul
to the list of windows being observed. maybe also the address book windows since that is also subject to Mozmill testing.

So the add-ons are bootstrapped now, they seem to load all the right JS files, but the test doesn't roll on. So we're missing something :-(
(Assignee)

Comment 19

a year ago
In fact, while the session is started and did nothing, I went into the add-ons manage and saw the add-ons there.
(Assignee)

Comment 20

a year ago
Posted patch 1449149-server.patch - WIP (obsolete) β€” β€” Splinter Review
Discussing this with Aceman via IRC we concluded that perhaps the server isn't started on time.

Previously it was started somehow via the command line in
jsbridge/extension/components/cmdarg.js

But we're not sure that mechanism still works. So I'm trying to start it here when JS Bridge starts, sadly with no success although the console log confirms the operation :-(
(Assignee)

Comment 21

a year ago
Actually, looking at the output, that gave:
Exception running bootstrap method startup on jsbridge@mozilla.com: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIAppShellService.hiddenDOMWindow]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://jsbridge/content/modules/server.js :: <TOP_LEVEL> :: line 48"  data: no] Stack trace: server.js:48

So this was too early ;-)
(Assignee)

Comment 22

a year ago
Posted patch 1449149-server.patch - WIP 2 (obsolete) β€” β€” Splinter Review
This is a little more successful, the server seems to be successfully started, however, at the end of the unsuccessful run I see:
Timeout: bridge.set("dd49e330-32d5-11e8-a398-2c56dc95cf0a", ChromeUtils.import('resource://mozmill/modules/mozmill.js'))
jsbridge.network.JSBridgeDisconnectError: Connection timed out

Hmm, another resource:// URL??
Attachment #8963384 - Attachment is obsolete: true
(Assignee)

Comment 23

a year ago
Posted patch 1449149-mozmill-move2.patch (obsolete) β€” β€” Splinter Review
Renaming resource:// in test files. Not really necessary for the moment.
(Assignee)

Comment 24

a year ago
Posted patch 1449149-server.patch - WIP 2b (obsolete) β€” β€” Splinter Review
Attachment #8963395 - Attachment is obsolete: true
(Assignee)

Comment 25

a year ago
Posted patch 1449149-Python.patch (obsolete) β€” β€” Splinter Review
(Assignee)

Comment 26

a year ago
Posted patch 1449149-mozmill-move3.patch (obsolete) β€” β€” Splinter Review
(Assignee)

Comment 27

a year ago
Posted patch 1449149-mozmill-move.patch (v3) (obsolete) β€” β€” Splinter Review
There was a reference to JS Bridge in Mozmill's modules/frame.js which I had missed.
Attachment #8963337 - Attachment is obsolete: true
(Assignee)

Comment 28

a year ago
Posted patch 1449149-Python.patch (obsolete) β€” β€” Splinter Review
Missed one :-(
Attachment #8963409 - Attachment is obsolete: true
(Assignee)

Comment 29

a year ago
With this heap of patches
  mozmake SOLO_TEST=composition/test-image-display.js mozmill-one
finally runs.

In the end I get: WARNING | endRunner was never called. There must have been a failure in the framework.

So we're getting close :-)
(Assignee)

Comment 30

a year ago
Posted patch 1449149-move.patch (obsolete) β€” β€” Splinter Review
Folded all the "move" patches into one, they were just boring.
Attachment #8963311 - Attachment is obsolete: true
Attachment #8963403 - Attachment is obsolete: true
Attachment #8963410 - Attachment is obsolete: true
Attachment #8963415 - Attachment is obsolete: true
Attachment #8963417 - Attachment is obsolete: true
Attachment #8963311 - Flags: feedback?(patrick)
(Assignee)

Comment 31

a year ago
This was pretty successful. Calendar tests fail, as expected, and theses ones as well:

content-tabs/test-content-tab.js
content-tabs/test-install-xpi.js (perhaps fails since test XPI isn't restartless)

folder-display/test-archive-messages.js
folder-display/test-deletion-with-multiple-displays.js
folder-display/test-message-commands.js
folder-display/test-right-click-middle-click-messages.js
folder-display/test-selection.js
folder-display/test-summarization.js
folder-display/test-tabs-simple.js
folder-display/test-tooltip-multimessage.js
folder-tree-modes/test-custom-folder-tree-mode.js
junk-commands/test-junk-commands.js

I'll see how to switch off calendar tests.
(Assignee)

Comment 33

a year ago
That didn't work, emptying out the list completely gives:
  tar: Cowardly refusing to create an empty archive
And it's downhill form there.
(Assignee)

Comment 35

a year ago
Posted patch 1449149-jsbridge.patch (v2) (obsolete) β€” β€” Splinter Review
Fixed comments.
(Assignee)

Comment 36

a year ago
Posted patch 1449149-mozmill.patch (v2) (obsolete) β€” β€” Splinter Review
Fixed comments.
Attachment #8963298 - Attachment is obsolete: true
Attachment #8963336 - Attachment is obsolete: true
(Assignee)

Comment 37

a year ago
Posted patch 1449149-server.patch (v3) (obsolete) β€” β€” Splinter Review
Fixed comment. Not WIP any more. I tried without and the bridge doesn't start.

So four patches are good to go here ;-)
Attachment #8963405 - Attachment is obsolete: true
(Assignee)

Comment 38

a year ago
Try from comment #34:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ee76d0245c1aa253942addd129ee4dff882b3583
shared-modules/test-calendar-utils.js is still run and fails, hmm.

Also failing: search-window/test-search-window.js

Comment 40

a year ago
Thanks for all the effort here guys!

With this series some mozmill tests pass. E.g. content-tabs/test-about-support.js still doesn't.
I'll comment on the patches now.

Updated

a year ago
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → 61
You may want to look into loading the default prefs for Mozimill and JsBridge. I don't think that they are currently loaded because extensionSupport.jsm only covers non-bootstrapped addons.

I'd add something like this, that gets called from startup():

// example with prefs for Mozmill:
function loadDefaultPrefs() {
  var prefSvc = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefService);
  var defaultBranch = prefSvc.getDefaultBranch(null);

  /* debugging prefs */
  defaultBranch.setBoolPref("browser.dom.window.dump.enabled", true);
  defaultBranch.setBoolPref("javascript.options.showInConsole", true);
}

Comment 42

a year ago
Comment on attachment 8963503 [details] [diff] [review]
1449149-jsbridge.patch (v2)

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

::: mail/test/resources/jsbridge/jsbridge/extension/bootstrap.js
@@ +7,5 @@
> +
> +  let wm = Cc["@mozilla.org/appshell/window-mediator;1"].getService(Ci.nsIWindowMediator);
> +
> +  // Wait for any new windows to open.
> +  wm.addListener(WindowListener);

If you import Services.jsm, you can use Services.wm here instead of defining 'wm'.

@@ +10,5 @@
> +  // Wait for any new windows to open.
> +  wm.addListener(WindowListener);
> +
> +  // Get the list of windows already open.
> +  let windows = wm.getEnumerator(null);

And here.

@@ +21,5 @@
> +
> +      domWindow.addEventListener("load", function loadUi() {
> +        domWindow.removeEventListener("load", loadUi, false);
> +        setupUI(domWindow);
> +      }, false);

If you use {once: true} argument here, the removeEventListener should be unneeded.

@@ +36,5 @@
> +}
> +
> +
> +function setupUI(domWindow) {
> +  var document = domWindow.document;

This seems unused.

@@ +62,5 @@
> +    domWindow.addEventListener("load", function listener() {
> +      domWindow.removeEventListener("load", listener, false);
> +
> +      setupUI(domWindow);
> +    }, false);

If you use {once: true} argument here, the removeEventListener should be unneeded.

@@ +78,5 @@
> +}
> +
> +function logException(exc) {
> +  try {
> +    ChromeUtils.import("resource://gre/modules/Services.jsm");

This would go, if you import Services at the top.
Attachment #8963503 - Flags: review+

Comment 43

a year ago
Comment on attachment 8963504 [details] [diff] [review]
1449149-mozmill.patch (v2)

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

The bootstrap.js file could use similar improvents as the jsbridge version.
Attachment #8963504 - Flags: review+

Updated

a year ago
Attachment #8963505 - Flags: review+

Comment 44

a year ago
(In reply to Patrick Brunschwig from comment #41)
> You may want to look into loading the default prefs for Mozimill and
> JsBridge. I don't think that they are currently loaded because
> extensionSupport.jsm only covers non-bootstrapped addons.

I'm not sure what kind of default prefs are not loaded yet, but the prefs that we initialize before TB starts under mozmill seem to be loaded. E.g. there are some predefined accounts in the profile. I can see them in the account manager test runs.

Updated

a year ago
Attachment #8963420 - Flags: review+
Under Mozmill there is a file "defaults/preferences/debug.js", containing the two prefs I mentioned. I expect they would need to be loaded, otherwise the file can be deleted entirely. AFAICT this changes the logging in TB.

Comment 46

a year ago
Posted patch 1449149-jsbridge.patch v3 (obsolete) β€” β€” Splinter Review
I have made the mentioned changes and merged this with the -server patch. I also add removal of the overlay.xul so that it is seen that is replaced by loading overlay.js in the window listeners in bootstrap.js.
Attachment #8963503 - Attachment is obsolete: true
Attachment #8963505 - Attachment is obsolete: true
Attachment #8963533 - Flags: review+

Comment 47

a year ago
Posted patch 1449149-mozmill.patch v3 (obsolete) β€” β€” Splinter Review
I have made the mentioned changes. I also add removal of the overlay.xul so that it is seen that is replaced by loading overlay.js in the window listeners in bootstrap.js.
Attachment #8963504 - Attachment is obsolete: true
Attachment #8963535 - Flags: review+

Comment 48

a year ago
Posted patch 1449149-move.patch v2 (obsolete) β€” β€” Splinter Review
I added the removal of the 'resource' directives in the manifest files so it is seen what this patch replaces. I also used the occassion and switched to double quotes in the import lines this patch touches.
Attachment #8963420 - Attachment is obsolete: true
Attachment #8963536 - Flags: review+

Comment 49

a year ago
Looking at the try run for why some tests failed, many of them end here:
INFO -    EXCEPTION: masks is undefined
INFO -      at: EventUtils.js line 190
INFO -         _parseModifiers EventUtils.js:190 5

'masks' is defined as 'const masks = Ci.nsIDOMNSEvent;'
Somehow nsIDOMNSEvent seems to be removed 6 years ago (bug 716822) and renamed to nsIDOMEvent.
It may be this path is only taken in some error condition so this leftover has been lurging there for ages (and also at other places in c-c).

I'll see if that moves us further.

Comment 50

a year ago
I have pushed the current patches and the nsIDOMNSEvent fix and calendar disabling to try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=84d16e46a892a692aacc36413649c85c22c67c4b

Comment 51

a year ago
(In reply to Jorg K (GMT+1) from comment #34)
> New try, replacing calendar tests with a dummy:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=ee76d0245c1aa253942addd129ee4dff882b3583

This still seems to run at least the tests that are directly in calendar/test/mozmill.
(Assignee)

Comment 52

a year ago
Thanks for integrating some clean-up!

I'm not sure why you based your improvements and merges on my old set of patches losing the fixed comments :-(
While you're improving, please rename setupUI() to setupServer() in the JS Bridge patch since there is no UI being initialised.

You might as well add reading the default values while we're here.

As for the test failures:
I saw this go past last night:
941b196e1e3b ui.manish β€” Bug 1429396 - Remove toolkit/content/global/XPCNativeWrapper.js r=mconley
https://dxr.mozilla.org/comm-central/search?q=XPCNativeWrapper&redirect=false
Maybe completely unrelated, but I though I'd mention it.
(Assignee)

Comment 53

a year ago
(In reply to :aceman from comment #51)
> This still seems to run at least the tests that are directly in
> calendar/test/mozmill.
We can deal with this later, see comment #38 and comment #39.

Comment 54

a year ago
This seems to help massively.

I'm not sure why this suddenly was a problem.
Maybe as Patrick says there is different logging due to not setting up some prefs.
Or now that the code isn't in a resource file we are no longer getting the existing shim at https://dxr.mozilla.org/comm-central/rev/56d6db4ad38c869d0bbc2aea449a4a382f109163/mozilla/dom/base/nsGlobalWindowInner.cpp#2911 which may have been hiding usage of this outdated interface.
Attachment #8963552 - Flags: review?(jorgk)
(Assignee)

Comment 55

a year ago
That last try looks really good. Perhaps you can work out how to squash the last calendar test an then inspect the failing:
content-tabs/test-install-xpi.js
folder-tree-modes/test-custom-folder-tree-mode.js

I'm sure the former is about the XPI not being bootstrapped. We know how to fix that now :-)
(Assignee)

Comment 56

a year ago
Comment on attachment 8963552 [details] [diff] [review]
1449149-DOMNSEvent.patch

Thanks, a lot of detective work?
Attachment #8963552 - Flags: review?(jorgk) → review+

Comment 57

a year ago
Posted patch 1449149-move.patch v3 β€” β€” Splinter Review
Convert also the references to moved mozmill files in the calendar tests otherwise it causes all those errors from mozmill/shared-modules/test-calendar-utils.js also in plain TB tests.
Attachment #8963536 - Attachment is obsolete: true
Attachment #8963554 - Flags: review+

Comment 58

a year ago
The calendar-utils warning is still there, it seems at line 10 it imports resource://calendar/modules/calUtils.jsm which is still at resourse:// and will not be solved by the patches here.

Comment 59

a year ago
(In reply to Jorg K (GMT+1) from comment #52)
> I'm not sure why you based your improvements and merges on my old set of
> patches losing the fixed comments :-(

Sorry, I thought I am working on the latest ones and didn't notice you also replaced the other ones, not just the server one that got upgraded from WIP.

> While you're improving, please rename setupUI() to setupServer() in the JS
> Bridge patch since there is no UI being initialised.

OK.

> You might as well add reading the default values while we're here.

I'll see.

Comment 60

a year ago
Updated the comments.
Attachment #8963533 - Attachment is obsolete: true
Attachment #8963632 - Flags: review+

Comment 61

a year ago
Updated the comments and added setting the default prefs.
Attachment #8963535 - Attachment is obsolete: true
Attachment #8963633 - Flags: review+
(Assignee)

Updated

a year ago
Keywords: leave-open

Comment 62

a year ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bf2506694f95
Make Mozmill add-on bootstrapped. r=aceman
https://hg.mozilla.org/comm-central/rev/553b84c7c6d5
Make JS Bridge add-on bootstrapped and start the internal server in add-on startup. r=aceman
https://hg.mozilla.org/comm-central/rev/e1ebe11b8359
JS Bridge: Move modules to chrome/content. Mozmill: Move modules and stdlib to content/. r=aceman
https://hg.mozilla.org/comm-central/rev/b6fefed16246
replace nsIDOMNSEvent with nsIDOMEvent in mozmill. r=jorgk CLOSED TREE DONTBUILD

Comment 63

a year ago
(In reply to :aceman from comment #40)
> E.g. content-tabs/test-about-support.js still doesn't.

This failed for me locally, but after I updated m-c to tip these pass too.
(Assignee)

Comment 64

a year ago
Posted patch 1449149-fix-xpi.patch β€” β€” Splinter Review
I added <em:bootstrap>true</em:bootstrap> to the RDF file in the XPI.
content-tabs/test-install-xpi.js passes with that.

Comment 65

a year ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/1c4e38199e99
change installxpi.xpi to bootstrapped. rs=bustage-fix CLOSED TREE DONTBUILD

Comment 66

a year ago
Posted patch 1449149-modeTest.patch (obsolete) β€” β€” Splinter Review
This makes the test-custom-folder-tree-mode.js going for me. It used an addon with overlay and non-bootstrapped.
Attachment #8963729 - Flags: review?(jorgk)
(Assignee)

Comment 67

a year ago
Comment on attachment 8963729 [details] [diff] [review]
1449149-modeTest.patch

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

Great!

::: mail/test/mozmill/folder-tree-modes/test-extension/bootstrap.js
@@ +34,5 @@
> +function shutdown(data, reason) {
> +  // Just ignore shutdowns.
> +}
> +
> +function setupUI(domWindow) {

Please find a better name, there is no UI setup here.

@@ +53,5 @@
> +                                                "Test Mode");
> +  }
> +
> +  if (domWindow.document.location.href ==
> +      "chrome://messenger/content/messenger.xul") {

Can that go on one line?
Attachment #8963729 - Flags: review?(jorgk) → review+

Comment 68

a year ago
Thanks.
Attachment #8963729 - Attachment is obsolete: true
Attachment #8963754 - Flags: review+

Updated

a year ago
Attachment #8963724 - Flags: review+
(Assignee)

Updated

a year ago
Keywords: leave-open

Comment 69

a year ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bbad34510f14
convert the extension for test-custom-folder-tree-mode.js into bootstrapped and without overlays. r=jorgk CLOSED TREE
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
(Assignee)

Updated

a year ago
Target Milestone: --- → Thunderbird 61.0
Note you can likely still register a resource:// uri manually using https://searchfox.org/comm-central/source/mozilla/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl#31 (which is the base interface for nsIResProtocolHandler)

Comment 71

a year ago
Comment on attachment 8963754 [details] [diff] [review]
1449149-modeTest.patch v1.1

And we can see how much boilerplate code was needed to simulate just a trivial overlay in an addon. Code that all addons will have to duplicate in differing ways. Way to go to dumb down the platform.
(Assignee)

Comment 72

a year ago
(In reply to :aceman from comment #71)
> Way to go to dumb down the platform.
You may want to implement bug 1450288 then. Also, could we remove |tearDownUI: function(window) {}| here:
https://hg.mozilla.org/comm-central/rev/bbad34510f140583fd3c0faac7d5b5558c8a2b99#l1.66
It slipped through the review.
Flags: needinfo?(acelists)

Comment 73

a year ago
Yes, also the onWindowTitleChange() seems not to be a member of nsIWindowMediatorListener.
Flags: needinfo?(acelists)
\o/ - thanks guys for sorting this out!!!
You need to log in before you can comment on or make changes to this bug.