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
Last year
Last year

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

Last year
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

Last year
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

Last year
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

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

Comment 6

Last year
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

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

Updated

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

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

Comment 24

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

Comment 25

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

Comment 26

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

Comment 27

Last year
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

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

Comment 29

Last year
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

Last year
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

Last year
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

Last year
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

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

Comment 36

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
Attachment #8963505 - Flags: review+

Comment 44

Last year
(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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
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

Last year
(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

Last year
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

Last year
(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

Last year
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

Last year
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

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

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

Comment 57

Last year
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

Last year
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

Last year
(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

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

Comment 61

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

Updated

Last year
Keywords: leave-open

Comment 62

Last year
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

Last year
(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

Last year
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

Last year
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

Last year
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

Last year
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

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

Updated

Last year
Attachment #8963724 - Flags: review+
Assignee

Updated

Last year
Keywords: leave-open

Comment 69

Last year
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
Closed: Last year
Resolution: --- → FIXED
Assignee

Updated

Last year
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

Last year
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

Last year
(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

Last year
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.