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)

RESOLVED FIXED in Thunderbird 3.3a1

Status

Thunderbird
Mail Window Front End
--
major
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

({regression})

Trunk
Thunderbird 3.3a1
regression
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
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

8 years ago
Bumping to "major" importance; this is causing spammy warning popups that effectively prevent leaving Thunderbird running.
Severity: normal → major
(Assignee)

Comment 2

8 years ago
Created attachment 456847 [details] [diff] [review]
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.
Attachment #456847 - Flags: review?(bienvenu)

Comment 3

8 years ago
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+
(Assignee)

Comment 4

8 years ago
(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.
(Assignee)

Comment 5

8 years ago
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.
status-thunderbird3.1: --- → .1-fixed
Flags: in-testsuite+
Whiteboard: [needs landing on trunk]
(Assignee)

Comment 6

8 years ago
Checked in on trunk: http://hg.mozilla.org/comm-central/rev/1577d14a133a
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing on trunk]
Target Milestone: --- → Thunderbird 3.2a1

Updated

4 years ago
See Also: → bug 577533
You need to log in before you can comment on or make changes to this bug.