1459743 - Port |Bug 1459245 - Migrate <stringbundle> from XBL binding to Custom Element to prove out the script loading plan for future Custom Elements|

Status: RESOLVED INVALID

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

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?

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1459743-stringbundle.patch

Aceman dictated me the patch via IRC ;-)

Comment 2

[:aceman]

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?

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1459743-stringbundle.patch (v2)

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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b00e830b9e5e
Port bug 1459245: Make sure stringbundle.js is loaded for all XUL documents. r=aceman

Comment 5

[Jorg K (CEST = GMT+2)]

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.

Comment 6

[Jorg K (CEST = GMT+2)]

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.

Comment 7

[Jorg K (CEST = GMT+2)]

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

Nothing to review, will be backed out.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7ce8558ff198
Backed out changeset b00e830b9e5e for being unnecessary. a=backout DONTBUILD

Comment 9

[Brian Grinstead [:bgrins]]

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.

Comment 10

[Jorg K (CEST = GMT+2)]

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

Comment 11

[John Bieling (:TbSync)]

Comment #7 indicates, that this has not made it into the product? It is at least not working with daily or current tip. I am verifing this for the addon update guide.

I think this is not needed at all, because calling Services.strings.createBundle(...) is much more simple than adding the stringbundle via overlay and then try to get it via getElementById(...).

I just need a status for the guide.

Comment 12

[Jorg K (CEST = GMT+2)]

Hmm, this was done in May 2018, I can't remember a thing. All I can say is that rev b00e830b9e5e was landed in comment #4 and backed out in comment #8.

Comment 13

[Jorg K (CEST = GMT+2)]

OK, nothing got fixed here, what was landed was backed out, so this looks like INVALID in the first place.