Closed Bug 1590074 Opened 5 years ago Closed 5 years ago

change the Thunderbird customElements.js loading to happen on document-element-inserted

Categories

(Thunderbird :: General, task, P1)

Product:
Thunderbird
Component:
General
Type:
task

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 72.0

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

Attachments

(2 files)

bug1590074_CE_document-element-inserted.patch
(deleted), patch
pmorris
: review+
Details | Diff | Splinter Review
bug1590074_followup_removals.patch
(deleted), patch
pmorris
: review+
Details | Diff | Splinter Review

At the time we set up customElements.js loading it was hooked up on chrome-document-global-created to make sure the toolkit customElements.js was loaded first.

This is not an issue anymore especially since toolkit customElements.js has protection from being loaded more than once.

We can clean this up, and also get the benefit that we don't have to do manual script includes for cases where chrome-document-global-created + DOMContentLoaded were too late.

Further I'll include some updates from toolkit to skip about:blank and apply the CEs to text/html documents too.

Comment on attachment 9102939 [details] [diff] [review] bug1590074_CE_document-element-inserted.patch Review of attachment 9102939 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Nice not to have to manually include that file. I spot checked it, opening up various windows and dialogs, and everything seems in working order. Nothing unusual in the console.
Attachment #9102939 - Flags: review?(paul) → review+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/30dd2e059b7d
change the Thunderbird customElements.js loading to happen on document-element-inserted. r=pmorris

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 72.0

I noticed some additional inclusions that should be removed in relation to this.
They do not cause any "real" problem, but you'll get an error message in the console about the custom element already being defined earlier.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fae1e36608f527d1b769232bf49796eff966ccc9

(The error of "NotSupportedError: 'conversation-browser' has already been defined as a custom element" seen on the preferences page is not related to this. That is due to the overlay loader and creation of a duplicate about:preferences document.)

Attachment #9103218 - Flags: review?(paul)
Comment on attachment 9103218 [details] [diff] [review] bug1590074_followup_removals.patch Review of attachment 9103218 [details] [diff] [review]: ----------------------------------------------------------------- r+ I didn't see the errors in the console with the patch (or without the patch for some reason...) when accessing the folder-menupopup CEs via the Go > Folder menu and same in the app/hamburger menu. Those things worked as expected showing we don't need these includes now.
Attachment #9103218 - Flags: review?(paul) → review+
Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/9bcb5b09c63b
follow-up to remove some CE widget includes no longer needed. r=pmorris

Keywords: checkin-needed
Regressions: 1591357
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: