Jorg K (CEST = GMT+2) Assignee
	Description • 6 years ago

According to bug 1459245 comment #3 we need to load stringbundle.js in all XUL documents.

M-C will do that in MainProcessSingleton.js with this new code, see:
https://reviewboard.mozilla.org/r/241726/diff/2#index_header

    case "document-element-inserted":
      // Set up Custom Elements for XUL windows before anything else happens
      // in the document. Anything loaded here should be considered part of
      // core XUL functionality. Any window-specific elements can be registered
      // via <script> tags at the top of individual documents.
      const doc = subject;
      if (doc.nodePrincipal.isSystemPrincipal &&
          doc.contentType == "application/vnd.mozilla.xul+xml") {
        for (let script of [
          "chrome://global/content/elements/stringbundle.js",
        ]) {
          Services.scriptloader.loadSubScript(script, doc.ownerGlobal);
        }
      }

Aceman, Richad, what's a good spot for this in C-C?

	Jorg K (CEST = GMT+2) Assignee
	Comment 1 • 6 years ago

Aceman dictated me the patch via IRC ;-)

	:aceman
	Comment 2 • 6 years ago

Comment on attachment 8973871 [details] [diff] [review]
1459743-stringbundle.patch

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

Also remove the observer in _dispose().
And how can we test this?

	Jorg K (CEST = GMT+2) Assignee
	Comment 3 • 6 years ago

For testing I applied bug 1459245:
https://hg.mozilla.org/integration/autoland/rev/80fc39b7c5ec

It seems to work, however, M-C forgot to cover the profile manager :-(
JavaScript error: chrome://mozapps/content/profile/profileSelection.js, line 41:
 TypeError: gProfileManagerBundle.getFormattedString is not a function

I also see:
JavaScript error: chrome://global/content/elements/stringbundle.js, line 61: Not
SupportedError: Operation is not supported

This will need to be pushed on the next M-C merge, potentially in the next three hours.

	Pulsebot
	Comment 4 • 6 years ago

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/b00e830b9e5e
Port bug 1459245: Make sure stringbundle.js is loaded for all XUL documents. r=aceman

	Jorg K (CEST = GMT+2) Assignee
	Comment 5 • 6 years ago

I still see:
JavaScript error: chrome://global/content/elements/stringbundle.js, line 62: Not Supported
Error: Operation is not supported

Apparently not a problem in FF. Brian suggested that we may not need this code here after all (bug 1459245 comment 25), so I'll investigate.

	Jorg K (CEST = GMT+2) Assignee
	Comment 6 • 6 years ago

OK, I added some debug to stringbundle.js 61:
console.trace("in stringbundle.js");
dump(window.location.toString()+"================\n");
customElements.define("stringbundle", MozStringbundle);
dump("after CE ================\n");

and for each XUL document I see:

console.trace: "in stringbundle.js"
chrome://global/content/elements/stringbundle.js 61
file:///C:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/components
/MainProcessSingleton.js 90 observe
chrome://messenger/content/newmailalert.xul================
after CE ================

So that does get called via MainProcessSingleton.js.

I'll back out our patch, not a lot of harm done, the script got loaded twice and we got the error from comment #5.

	Jorg K (CEST = GMT+2) Assignee
	Comment 7 • 6 years ago

Comment on attachment 8973932 [details] [diff] [review]
1459743-stringbundle.patch (v2)

Nothing to review, will be backed out.

	Pulsebot
	Comment 8 • 6 years ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/7ce8558ff198
Backed out changeset b00e830b9e5e for being unnecessary. a=backout DONTBUILD

	Brian Grinstead [:bgrins]
	Comment 9 • 6 years ago

So <stringbundle>s are working properly now? If so, that's good news since other binding migrations should happen transparently unless if frontend changes are required as part of the migration.

	Jorg K (CEST = GMT+2) Assignee
	Comment 10 • 6 years ago

(In reply to Brian Grinstead [:bgrins] from comment #9)
> So <stringbundle>s are working properly now?
Yes, thanks.