Closed Bug 682583 Opened 13 years ago Closed 13 years ago

Remove calendar's dependency on nsTryToClose.js

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: standard8, Assigned: standard8)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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.
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?
Oh, you did. So in that case its just add their code for the event dialog.
Attached patch The fix (obsolete) — Splinter Review
This should fix it.
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #567049 - Flags: review?(philipp)
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.
Attachment #567049 - Flags: review?(philipp) → review+
(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.
Attached patch The fix v2Splinter Review
With the missing part of the diff.
Attachment #567049 - Attachment is obsolete: true
Attachment #567401 - Flags: review+
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.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.