Closed Bug 341957 Opened 19 years ago Closed 19 years ago

[patch] Improved mail alert does not respect task bar position

Categories

(Thunderbird :: Mail Window Front End, defect)

x86
Windows XP
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird2.0

People

(Reporter: jens.b, Assigned: jens.b)

References

Details

(Keywords: fixed1.8.1.2)

Attachments

(1 file)

It would look silly if we shipped a rewritten, significantly improved new mail alert that is hard-coded to appear at the bottom right corner of the screen and thus still suffers from bug 133527 (filed four years ago). As my patch for that bug will be ready within the next few weeks and the required changes for the new mail alert are pretty simple, I would like to get this into Thunderbird 2.0. What sort of coordination, approvals, blocker flags and/or driver decisions would be neccessary here? And how does the feature freeze by Firefox beta affect code (toolkit and widget) shared by Thunderbird, which is not yet in beta stage?
Requesting blocker flag to make sure this gets noticed :-)
Flags: blocking-thunderbird2?
Changing title to make clear that this is not a duplicate. Bug 133527 targets the alerts service which is no longer used by Thunderbird on Windows.
Summary: New mail alert does not respect task bar position → Improved mail alert does not respect task bar position
Attached patch use alert originSplinter Review
Let the mail alert use the alert origin reported by the look and feel, if available. Combined with my patch for bug 133527, this works fine on trunk. Note that I did not make any layout adaptions for the alert itself when it's anchored to a left/right anchored task bar, i.e. the alert is still in landscape format. If desired, I could add that as well by moving the Thunderbird icon below or above the text (like I've done with the generic alerts service).
Attachment #226383 - Flags: superreview?
Attachment #226383 - Flags: review?
Attachment #226383 - Flags: superreview?(mscott)
Attachment #226383 - Flags: superreview?
Attachment #226383 - Flags: review?(mscott)
Attachment #226383 - Flags: review?
Summary: Improved mail alert does not respect task bar position → [patch] Improved mail alert does not respect task bar position
Target Milestone: --- → Thunderbird2.0
Comment on attachment 226383 [details] [diff] [review] use alert origin Thanks Jens. This looks pretty good. Do you need me to land it for you?
Attachment #226383 - Flags: superreview?(mscott)
Attachment #226383 - Flags: superreview+
Attachment #226383 - Flags: review?(mscott)
Attachment #226383 - Flags: review+
(In reply to comment #4) > Thanks Jens. This looks pretty good. Do you need me to land it for you? Unfortunately, this has to wait for bug 133527 which introduces the alert origin look & feel value. Which leads to my question in comment 0 - what can we do so both bugs make it in on time? I need a review there (Neil told me to get someone else to look at this too), and the patch touches widget and toolkit. Can you raise attention for that bug?
Depends on: 133527
Whiteboard: [checkin needed]
mozilla/mail/base/content/newmailalert.js 1.13 mozilla/mailnews/base/src/nsMessengerWinIntegration.cpp 1.63
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Comment on attachment 226383 [details] [diff] [review] use alert origin - Has been on trunk since 2006-07-19, without regressions - Change summary: Changes Windows messenger integration to get the new alert origin value from the look and feel. Changes the improved mail alert window to position itself according to that value. - Risk assessment: very low for both JS and C++ code - the patch only adds new behaviour, and is very small. - L10N impact: no string changes. - No API is changed. - Longstanding bug with high visibility for users with non-bottom-taskbars (mail alert is original motivation for bug 133527 from 2002) - [Depends on bug 133527 also being approved]
Attachment #226383 - Flags: approval1.8.1?
Comment on attachment 226383 [details] [diff] [review] use alert origin Drivers feel like it's too late to be taking this change on the branch for Firefox2 at this stage in the game.
Attachment #226383 - Flags: approval1.8.1? → approval1.8.1-
minusing as a tb2 blocker since it didn't make it past the 1.8.1 triage team.
Flags: blocking-thunderbird2? → blocking-thunderbird2-
Comment on attachment 226383 [details] [diff] [review] use alert origin Drivers approved bug 133527 for TB2 (1.8.1.2) - however, in itself, that bug won't benefit Thunderbird 2 at all. In order to make use of that code for the improved mail alert, this patch must go in, too. Change impact and risk assessment: minimal, patch consists of ~35 lines well baked on the trunk.
Attachment #226383 - Flags: approval1.8.1.2?
Comment on attachment 226383 [details] [diff] [review] use alert origin Approved for 1.8 branch, a=jay for drivers.
Attachment #226383 - Flags: approval1.8.1.2? → approval1.8.1.2+
Gavin, could you check in attachment 226383 [details] [diff] [review] to the 1.8 branch? It applies with fuzz=5. Bug 133527 needs to go in first.
Whiteboard: [checkin needed (1.8 branch)]
mozilla/mailnews/base/src/nsMessengerWinIntegration.cpp 1.53.2.5 mozilla/mail/base/content/newmailalert.js 1.4.2.12
Keywords: fixed1.8.1.2
Whiteboard: [checkin needed (1.8 branch)]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: