Closed Bug 1490269 Opened Last year Closed Last year

enable thunderbird specific custom element loading

Categories

(Thunderbird :: Mail Window Front End, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 64.0

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

Attachments

(1 file, 2 obsolete files)

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

Patch coming.
This seems to do it for me. 

I refreshed the bug 1489748 patch to apply on top of this to test it out
Attachment #9008029 - Flags: review?(arshdkhn1)
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?
Attachment #9008029 - Flags: review?(arshdkhn1) → review-
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
Attachment #9008029 - Flags: review- → review?(arshdkhn1)
(And yes, working well here.)
Attachment #9008029 - Flags: review?(arshdkhn1) → review+
Keywords: checkin-needed
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.
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.
Keywords: checkin-needed
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.
Attachment #9008029 - Attachment is obsolete: true
Attachment #9008861 - Flags: review+
Keywords: checkin-needed
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
Toolkit style
(must have been copy-paste error)
Attachment #9008861 - Attachment is obsolete: true
Attachment #9008865 - Flags: review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/760fa56f2806
enable Thunderbird specific custom element loading. r=arshad
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.