1538509 - [de-xbl] remove the overlayTrigger binding

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Magnus Melin [:mkmelin]]

Remove the overlayTriger binding.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1538509_dexbl_overlayTrigger.patch

Looking at this more closely, I'm not sure what it's supposed to do.

It creates an element, then appends that to the document. Loads the css for the xbl, so the element would get binding attached. That causes the custom event, which the event listener reacts to.

Unless there's some side effect I don't see here, it looks rather pointless, and more of a game to win some time?

At least lightning seems ok with this.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3424f6971806038e49f03f00e1afa0cba1aca371

Comment 2

[Geoff Lankow (:darktrojan)]

I have a bad feeling about doing this. What that code is supposed to do is delay the fake load event we send to listeners in the event of an overlay being applied after the real load event, which happens in some circumstances. We wait for a stylesheet to load and a binding to be constructed so that we can assume all stylesheets have loaded and all bindings have been applied.

I'm going to make a build and try this on one of my VMs, as it used to run this bit of code frequently.

Comment 3

[Magnus Melin [:mkmelin]]

I guess the question is, why do we need to wait for all stylesheets or bindings to be applied? If it's purely about bindings (which would be applied through stylesheet hookup) then for any overlay loading this is just not something we'd support anymore in 68. For 68 add-ons xbl bindings can be considered dead, and there's hope we're not really even using them in the core product for anything.

Comment 4

[Geoff Lankow (:darktrojan)]

Comment on attachment 9053070 [details] [diff] [review]
bug1538509_dexbl_overlayTrigger.patch

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

Oh go on then. I suppose it's only a tiny fraction of use cases even if it was doing something useful.

Comment 5

[Magnus Melin [:mkmelin]]

We might want to delay landing it until calendar xbl is further done, just to be sure.

Comment 6

[Wayne Mery (:wsmwk)]

This is engineering, so => task https://wiki.mozilla.org/BMO/UserGuide/BugFields#bug_type

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1538509_dexbl_overlayTrigger.patch

Lets get this landed. Good try at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c2e4cbe9650b6e90e5a73b303c9694994a802293

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/44e0b7bccc36
[de-xbl] remove the overlayTrigger binding. r=darktrojan

Comment 9

[Jorg K (CEST = GMT+2)]

This is really a Calendar bug fixed at 7.1.