1490269 - enable thunderbird specific custom element loading

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, enhancement)

Description

[Magnus Melin [:mkmelin]]

We need a way to load thunderbird's custom elements. Like for bug 1489748.

Patch coming.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1490269_tb_customelements.patch

This seems to do it for me. 

I refreshed the bug 1489748 patch to apply on top of this to test it out

Comment 2

[Arshad Khan [:arshad]]

Comment on attachment 9008029 [details] [diff] [review]
bug1490269_tb_customelements.patch

Hey, the if condition in mailglue.js doesn't get truthy so the custom elements file is never called. My working directory is old, but i dont think that's the reason behind this. Could you recheck on your side and flag me again for review?

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9008029 [details] [diff] [review]
bug1490269_tb_customelements.patch

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

There's really no reason those wouldn't get true. It's all copied from https://searchfox.org/comm-central/source/mozilla/toolkit/components/processsingleton/CustomElementsListener.jsm which loads the m-c customELements.js

THis patch in itself doesn't load the file of course, it's just infra. 
You need the updated patch from bug 1489748 on top for it to do anything

Comment 4

[Magnus Melin [:mkmelin]]

(And yes, working well here.)

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 9008029 [details] [diff] [review]
bug1490269_tb_customelements.patch

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

Did you lint this?

::: mail/components/mailGlue.js
@@ +132,5 @@
> +
> +      // Set up our custom elements.
> +      aSubject.addEventListener("DOMDocElementInserted", () => {
> +        let doc = aSubject.document;
> +        if (doc.nodePrincipal.isSystemPrincipal && (

This is really unusual placement of the parenthesis. It should go onto the next line.

@@ +135,5 @@
> +        let doc = aSubject.document;
> +        if (doc.nodePrincipal.isSystemPrincipal && (
> +            doc.contentType == "application/vnd.mozilla.xul+xml" ||
> +            doc.contentType == "application/xhtml+xml"
> +          )) {

Please put at the end of the previous line.

Comment 6

[Jorg K (CEST = GMT+2)]

c:\mozilla-source\comm-central\comm\common\content\customElements.js
  9:1  error  Block is redundant.  no-lone-blocks (eslint)

If you want the block as the comment says, please disable linting there.

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1490269_tb_customelements.patch

Eslint is actually correct. Since we don't do any declarations in the block it's not necessary.

The braces style was just copied from the toolkit file per above. Tried changing them now, but the original is easier to read so I didn't change it.

Comment 8

[Jorg K (CEST = GMT+2)]

Hmm, but in the end you didn't follow toolkit's style but made up your own :-(

If you really must place the closing parenthesis of the 'if' on a new line, it goes right under the 'if'.
https://searchfox.org/mozilla-central/rev/dd965445ec47fbf3cee566eff93b301666bda0e1/toolkit/components/processsingleton/CustomElementsListener.jsm#17

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: bug1490269_tb_customelements.patch

Toolkit style
(must have been copy-paste error)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/760fa56f2806
enable Thunderbird specific custom element loading. r=arshad