682580 - Remove suite's dependency on nsTryToClose.js

Status: RESOLVED FIXED

(SeaMonkey :: General, 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.

Comment 1

[Mark Banner (:standard8)]

Note: I've said we'll do this before the next merge point, so they can land their patch before then. It should be simple enough to do before then.

Comment 2

[[e-mail]]

Attachment: Editor fix

* Turns EditorCanClose into an nsIObserver function (fortunately we don't have to use objects these days) that can cancel the quit request
* Adds it in EditorStartup rather than EditorOnLoad so that the plaintext editor can prompt on quit request as well (and also so that we can remove it!)
* Removes it in EditorShutdown to avoid leaks

Comment 3

[[e-mail]]

Attachment: Editor fix [Checked in: Comment 10 & 11]

* Added check to see whether quit has already been cancelled.

Comment 4

[Ian Neal]

Attachment: Rest of Suite fix

Following a similar tack to editor patch

Comment 5

[[e-mail]]

Comment on attachment 568245 [details] [diff] [review]
Rest of Suite fix

>-    return false;
>+    canClose = false;
>   }
> 
>   // Returns FALSE only if user cancels save action
>-  if (gContentChanged || gMsgCompose.bodyModified || (gAutoSaveKickedIn && !gEditingDraft))
>+  if (canClose && (gContentChanged || gMsgCompose.bodyModified ||
>+                   (gAutoSaveKickedIn && !gEditingDraft)))
This particular function is sufficiently complex that it might be worth leaving it and reusing the (conveniently) existing observer to handle all the quit-related code, along the lines of
case "quit-application-requested":
  if (!aSubject.data)
    aSubject.data =!ComposeCanClose();

>     if (Services.prompt.confirmEx(window, promptTitle, promptMsg,
>         (Services.prompt.BUTTON_TITLE_IS_STRING *
>          Services.prompt.BUTTON_POS_0) +
>         (Services.prompt.BUTTON_TITLE_IS_STRING * Services.prompt.BUTTON_POS_1),
>         continueButtonLabel, stopButtonLabel, null, null, {value:0}))
>       gFilterListMsgWindow.StopUrls();
>-    else
>+    else {
>+      if (!reallyClose && aTopic == "quit-application-requested")
>+        aCancelQuit.data = true;
>       return false;
>+    }
This return-in-else block is really against our coding style, the original code should have said something along the lines of
if (Services.prompt.confirmEx(...) == 0)
  return false;
gFilterListMsgWindow.StopUrls();

Comment 6

[Ian Neal]

Attachment: Rest of Suite fix v1.1 [Checked in: Comment 8]

Changes since last version:
* Use existing observer in MsgComposeCommands.js
* Switch if clause around in FilterListDialog.js

Comment 7

[[e-mail]]

Comment on attachment 568559 [details] [diff] [review]
Rest of Suite fix v1.1 [Checked in: Comment 8]

[Existing behaviour that I noticed: you can only "Don't Save" a message once; if you cancel the quit for some other reason, the message window doesn't prompt you next time you close or quit. Other windows prompt every time.]

>+    else if (aTopic == "quit-application-requested" &&
>+             aSubject instanceof Components.interfaces.nsISupportsPRBool &&
>+             !aSubject.data)
>+      aSubject.data = !ComposeCanClose();
[I also came up with a slightly different version, but I can't decide:
else if (aTopic == "quit-application-requested" &&
         aSubject instanceof Components.interfaces.nsISupportsPRBool &&
         !aSubject.data &&
         !ComposeCanClose())
  aSubject.data = true;
]

Two actual bugs though; r=me with these fixed:
>-  }
Compose window fails to load because of this deletion.

>+      if (!reallyClose && aTopic == "quit-application-requested")
The filter dialog doesn't use reallyClose.

Comment 8

[Ian Neal]

Comment on attachment 568559 [details] [diff] [review]
Rest of Suite fix v1.1 [Checked in: Comment 8]

http://hg.mozilla.org/comm-central/rev/8e21a323a4bb

Comment 9

[Ian Neal]

(In reply to [e-mail] from comment #7)
> [Existing behaviour that I noticed: you can only "Don't Save" a message
> once; if you cancel the quit for some other reason, the message window
> doesn't prompt you next time you close or quit. Other windows prompt every
> time.]
Spun off to bug 698011

Comment 10

[Ian Neal]

Comment on attachment 568196 [details] [diff] [review]
Editor fix [Checked in: Comment 10 & 11]

http://hg.mozilla.org/comm-central/rev/a5bfc8f64c86
http://hg.mozilla.org/comm-central/rev/1cc58919c01a
http://hg.mozilla.org/comm-central/rev/278f17d8e667

Comment 11

[Ian Neal]

Comment on attachment 568196 [details] [diff] [review]
Editor fix [Checked in: Comment 10 & 11]

http://hg.mozilla.org/comm-central/rev/b06edef5f722