682583 - Remove calendar's dependency on nsTryToClose.js

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Mark Banner (:standard8)]

Firefox is removing nsTryToClose.js from Firefox because it registers as app-startup and hence slows down starting up.

We should just switch these to observer registrations in appropriate places.

Someone fancy taking this on? I've said we'll do this before the next merge point, so they can land their patch before then.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Could you reference the firefox bug on this? I assume we just need to duplicate the registration code for the event dialog window, but maybe its just a matter of instanciating the appshell/appStartup component there?

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Oh, you did. So in that case its just add their code for the event dialog.

Comment 3

[Mark Banner (:standard8)]

Attachment: The fix

This should fix it.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 567049 [details] [diff] [review]
The fix

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

r=philipp

::: calendar/base/content/dialogs/calendar-event-dialog.js
@@ +42,4 @@
>   * ***** END LICENSE BLOCK ***** */
>  
>  Components.utils.import("resource://calendar/modules/calUtils.jsm");
> +Components.utils.import("resource://gre/modules/Services.jsm");

Wasn't there something about not using gre but instead resource:///modules/Services.jsm? I don't quite remember, so please ignore if irrelevant.

@@ +257,4 @@
>      document.documentElement._hitEnter = function() {};
>  }
>  
> +function onEventDialogUnload() {

I assume this needs to be referenced somewhere, i.e xul file.

Comment 5

[Mark Banner (:standard8)]

(In reply to Philipp Kewisch [:Fallen] from comment #4)
> Comment on attachment 567049 [details] [diff] [review] [diff] [details] [review]
> The fix
> 
> Review of attachment 567049 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> r=philipp
> 
> ::: calendar/base/content/dialogs/calendar-event-dialog.js
> @@ +42,4 @@
> >   * ***** END LICENSE BLOCK ***** */
> >  
> >  Components.utils.import("resource://calendar/modules/calUtils.jsm");
> > +Components.utils.import("resource://gre/modules/Services.jsm");
> 
> Wasn't there something about not using gre but instead
> resource:///modules/Services.jsm? I don't quite remember, so please ignore
> if irrelevant.

A quick mxr shows we're using gre pretty much everywhere.

> @@ +257,4 @@
> >      document.documentElement._hitEnter = function() {};
> >  }
> >  
> > +function onEventDialogUnload() {
> 
> I assume this needs to be referenced somewhere, i.e xul file.

Yep, I forgot to qrefresh before attaching.

Comment 6

[Mark Banner (:standard8)]

Attachment: The fix v2

With the missing part of the diff.

Comment 7

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/6feb8945d4e0

Let me know if you want this transplanted across. I don't think it is necessary as the dependent change won't go in on branches.