Closed Bug 332160 Opened 19 years ago Closed 18 years ago

Hook up new mail alert notifications for UNIX

Categories

(Thunderbird :: Mail Window Front End, defect)

Product:
Thunderbird
Component:
Mail Window Front End
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird2.0

People

(Reporter: mscott, Assigned: mkmelin)

References

Details

(Keywords: fixed1.8.1.1)

Attachments

(3 files, 3 obsolete files)

screenshot with css fix - new mail notification in linux/kde
20.27 KB, image/png
Details
proposed fix, v2
8.30 KB, patch
mscott
: review+
mscott
: superreview+
Details | Diff | Splinter Review
the backed out branch changes with problem fixed
6.49 KB, patch
mscott
: review+
mscott
: superreview+
mscott
: approval-thunderbird2+
Details | Diff | Splinter Review
Neil just did some work for the mail notification in seamonkey (see Bug 327613) to make it work for UNIX builds. We should add this to the build for Thunderbird and expose the pref UI to control it.
actually it looks like we don't hide the show an alert pref in the options UI for unix, so the UI is already there! This might be as easy as just adding Neil's new file to the Thunderbird build.
OS -> linux :)
OS: Windows XP → Linux
Magnus, are you on linux? If so, do you want to hook this up? Should be pretty straight forward.
Working on it...
Attached patch proposed fix (obsolete) — — Splinter Review
This is mostly copy-paste from the windows integration, making unix tb use the same alert as on windows.
Assignee: mscott → mkmelin+bmo
Status: NEW → ASSIGNED
Attachment #218189 - Flags: review?(mscott)
Comment on attachment 218189 [details] [diff] [review] proposed fix awesome. How does it look on unix? Does the existing pref UI work ok for turning it off? I can land this on the trunk and branch if you need me too.
Attachment #218189 - Flags: superreview+
Attachment #218189 - Flags: review?(mscott)
Attachment #218189 - Flags: review+
> How does it look on unix? Does the existing pref UI work ok for turning it off? As you can see in the screenshot, the close notification x is cut off on top by 1px or so for me, I don't know if it's something with my system...? But other than that, I think it looks quite nice! The pref works fine. Yep, do check it in, I don't have cvs checkin rights.
Hmm, I wonder why the blue border and the white background aren't showing up. Some of the style rules don't seem to be working compared to the windows version: http://www.mozilla.org/projects/thunderbird/specs/images/newmailalert.png
does dom inspector show anything interesting in the window that would explain why the blue border is missing? The border style is coming from: http://lxr.mozilla.org/mozilla/source/mail/themes/qute/mail/newmailalert.css#47
Ok, I got it working... Possible patch coming up.
Attachment #218673 - Attachment is obsolete: true
This patch fixes the color issues for linux. Hope it works for windows too. I'm not sure what the problem was. Earlier, I was able to change fonts and stuff, just not the border-color and background-color. I think the border was actually there, only not the in correct color. At least I was able to change it's width.
Attachment #219340 - Flags: review?(mscott)
Comment on attachment 219340 [details] [diff] [review] proposed additional fix, make the alert look like on windows -- does not work on win Very strange, this doesn't work on windows. The styles need to be on the window element not the stack for Windows, but on the stack element for Linux? How odd.
Rob, this is the bug that's going to add the nice alert notification to linux that I was telling you about. nsMessengerUnixIntegration.cpp is where we would also want to add code to add a system tray icon.
Attachment #219340 - Attachment description: proposed additional fix, make the alert look like on window → proposed additional fix, make the alert look like on windows -- does not work on win
Attachment #219340 - Flags: review?(mscott)
*** Bug 342210 has been marked as a duplicate of this bug. ***
jens, can you help us figure out why the first patch in this bug doesn't seem to look right on linux?
(In reply to comment #15) > jens, can you help us figure out why the first patch in this bug doesn't seem > to look right on linux? I cannot see anything obvious right now. I will have a more detailed look at it this weekend, though.
Yes! I hope this makes Thunderbird 2.0 (or come on a fix for 1.5). I've been missing out on new mail alerts since I've moved to Linux from Windows.
Does this alert rely on a system tray? How about window managers that don't have one (eg. WindowMaker). Ideally I would like to see the application icon change for the minimised window if I have unread mail (not new mail), or if this is not possible change the window title to 'unread'. This means I can set up mail filters so I don't get notified and disrupted for new spam etc.
Jens, do you have any time to help shed some light on this?
(In reply to comment #19) > Jens, do you have any time to help shed some light on this? Scott, I can't explain the win32/unix difference in styles at all. I guess this is some oddity in widget implementations (not that I know any details in that area). You should ask some widget folks for explanations and/or workarounds. If the behaviour cannot be circumvented or fixed, we could use the chrome registry's flags for win/unix/mac here to include different CSS files. Perhaps #ifdef'ing the relevant style rules between both platforms is also possible.
Ok, let's do this for now. Unfortunately we can't use the xul pre-processor on CSS files because the # used for element ids confuses the pre-processor. But we have a couple options. Option A: 1) Add a new theme file newMailAlertUnix.css in mail\themes\qute\mail 2) Add the styles for #alertContainer 3) we can ifdef newMailAlert.xul to include newMailAlertUnix.css for unix Option B: If that doesn't work, we can fork newMailAlert.css all together and have a copy that gets loaded for windows and another version that gets loaded for unix. Jens or Magnus, do one of you want to try out option A? I don't think it will be too hard and I'll gladly help get it checked in if it works.
*** Bug 360545 has been marked as a duplicate of this bug. ***
Attached patch proposed fix, v2 — — Splinter Review
This is basically the same patch as before + "Option A". Applying it will make the UNIX alert look like in attachment 219336 [details].
Attachment #218189 - Attachment is obsolete: true
Attachment #219340 - Attachment is obsolete: true
Attachment #246611 - Flags: superreview?(mscott)
Attachment #246611 - Flags: review?(mscott)
Comment on attachment 246611 [details] [diff] [review] proposed fix, v2 awesome thanks for doing this.
Attachment #246611 - Flags: superreview?(mscott)
Attachment #246611 - Flags: superreview+
Attachment #246611 - Flags: review?(mscott)
Attachment #246611 - Flags: review+
I just checked this into the trunk. The unix mail notification class added for bug 327613 isn't on the branch so we need to land that first.
Depends on: 327613
(In reply to comment #25) > I just checked this into the trunk. > > The unix mail notification class added for bug 327613 isn't on the branch so we > need to land that first. > I'm being stupid. 327613 is indeed on the branch.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
see regression crash bug 362225 on today's Tb 2.0a1 nightly. Rollback?
(In reply to comment #27) > see regression crash bug 362225 on today's Tb 2.0a1 nightly. Rollback? > Oops. 2.0b1, namely "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1pre) Gecko/20061129 Thunderbird/2.0b1pre" - Build ID: 2006112903
I've backed this out of the branch where it was causing that crash. It's still checked into the trunk.
Keywords: fixed1.8.1.1
Depends on: 362225
(In reply to comment #29) > I've backed this out of the branch where it was causing that crash. It's still > checked into the trunk. > "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1pre) Gecko/20061130 Thunderbird/2.0b1pre" - Build ID: 2006113004 Crash has indeed disappeared. Alert again can't be displayed regardless of "Edit => Preferences => General => When new messages arrive, [x] Show an alert" (just like it was until day before yesterday).
Magnus, what happens if you back out say just the unix specific CSS changes on the branch, leaving the alert itself running with broken CSS, does is still crash? Just trying to isolate the specific thing that's triggering the crash since the stack traces seem random.
This seems to fix it... I'm no longer crashing after I removed the unix specific rules for min-height and background-color on #newMailAlertNotification. Turns out they were not necessary.
Attachment #247444 - Flags: superreview?(mscott)
Attachment #247444 - Flags: review?(mscott)
Comment on attachment 247444 [details] [diff] [review] the backed out branch changes with problem fixed thanks for the extra effort Magnus.
Attachment #247444 - Flags: superreview?(mscott)
Attachment #247444 - Flags: superreview+
Attachment #247444 - Flags: review?(mscott)
Attachment #247444 - Flags: review+
Attachment #247444 - Flags: approval-thunderbird2+
I've landed this new branch patch on the branch.
Keywords: fixed1.8.1.1
*** Bug 315387 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: