577343 - Errors/warnings should only be alerted if there is a msgWindow associated (i.e. as a result of a user action rather than a background action)

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mark Banner (:standard8)]

When we swapped to non-modal alerts we accidentally lost the check for a msgWindow when displaying the alert. This means we show more alerts than we were intending.

We need to stop this happening and move back to only alerting if we have a msgWindow associated, i.e. as a result of user action rather than a background action.

Comment 1

[Scott A. Colcord]

Bumping to "major" importance; this is causing spammy warning popups that effectively prevent leaving Thunderbird running.

Comment 2

[Mark Banner (:standard8)]

Attachment: The fix

When I looked at the c++ code for this, I realised this was going to be simpler than I originally thought :-)

So here's the patch and a unit test for it.

Comment 3

[David :Bienvenu]

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

typo in comment:
+    // nsIMsgMailNewsUrl.msgWdindow will throw on a null pointer, so that's
+    // what we're handling here.

are other kinds of exceptions OK, or is this just an abundance of caution?

+    catch (ex if (ex instanceof Ci.nsIException &&
+                  ex.result == Cr.NS_ERROR_INVALID_POINTER)) {
+      return true;

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> are other kinds of exceptions OK, or is this just an abundance of caution?
> 
> +    catch (ex if (ex instanceof Ci.nsIException &&
> +                  ex.result == Cr.NS_ERROR_INVALID_POINTER)) {
> +      return true;

I was being cautious - we have a lot of places where we catch errors we shouldn't do (and suffer as a result), plus it should be the only error expected to be returned by that function, so if there's others, we should know about them.

Comment 5

[Mark Banner (:standard8)]

As trunk is closed, and this is needed for .1 I've landed on comm-1.9.2 default and relbranch:

http://hg.mozilla.org/releases/comm-1.9.2/rev/01ddbab92467
http://hg.mozilla.org/releases/comm-1.9.2/rev/96c512077045

Leaving open until we fix this on trunk as well.

Comment 6

[Mark Banner (:standard8)]

Checked in on trunk: http://hg.mozilla.org/comm-central/rev/1577d14a133a