Closed Bug 577343 Opened 9 years ago Closed 9 years ago

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)

Categories

(Thunderbird :: Mail Window Front End, defect, major)

defect
Not set
major

Tracking

(blocking-thunderbird3.1 .1+, thunderbird3.1 .1-fixed)

RESOLVED FIXED
Thunderbird 3.3a1
Tracking Status
blocking-thunderbird3.1 --- .1+
thunderbird3.1 --- .1-fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
Bumping to "major" importance; this is causing spammy warning popups that effectively prevent leaving Thunderbird running.
Severity: normal → major
Attached patch The fixSplinter Review
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.
Attachment #456847 - Flags: review?(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;
Attachment #456847 - Flags: review?(bienvenu) → review+
(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.
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.
Flags: in-testsuite+
Whiteboard: [needs landing on trunk]
Checked in on trunk: http://hg.mozilla.org/comm-central/rev/1577d14a133a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing on trunk]
Target Milestone: --- → Thunderbird 3.2a1
See Also: → 577533
You need to log in before you can comment on or make changes to this bug.